dah - matthias, i'm not sure why the default method helps now! :( unless I've forgotten something, it doesn't seem to solve very much at all. so again....dah!. leaving it up to the graph implementation seems like a problem to me though. i guess we could simply document the implementation of re-attaching a Property to a Graph is to use the default implementation property(k,v). If the user wants something else, they write their own attach Function<Attachable<V>, V>. That doesn't solve the problem of properly writing tests though because I guess it could mean different assertions - maybe that can be mitigated somehow. Marko, do you have anything else to add to this?
matt, note that i was referring to Element.property(k,v) not a property() method on a Traversal. I'm not sure if that changes your opinions in any way or not, but I like the property(k,v) convenience method. I don't think it's something we should remove. This issue I've raised is more a question of how we internally make calls to set a property internally in the stack when we don't know what cardinality the user will want. What if we added Graph.features().vertex().getDefaultCardinality()? Then I could stick to calling property(k,v) and then easily assert the right things in tests. On Fri, May 1, 2015 at 3:17 PM, Matthias Broecheler <[email protected]> wrote: > Hi Stephen, > > sorry, I don't understand why you need to have a default implementation in > gremlin-core? Why not just call property(key,value) and leave it up to the > graph implementation to handle how that is implemented? A default method > implementation is just a convenience that Java affords - I missing the > piece where this becomes a necessity for you. > > Thanks, > Matthias > > On Fri, May 1, 2015 at 5:35 AM Stephen Mallette <[email protected]> > wrote: > > > Sorry to be dredging up some old business with respect to the settled > issue > > of the default cardinatlity of property(k,v) which we some time ago voted > > to leave up to vendors. We further decided to make sure that in the > tests > > we always called property(cardinality, k, v) and use features to > > appropriately filter the test suite when executed by vendors. > > > > What that didn't address was how we internally utilize property(k,v) > within > > gremlin-core. The core example here would be with IO where we want to > > provide internal "getOrCreate" functionality. What should that > > "getOrCreate" do on create for properties? If we were to explicitly > call: > > > > property(list, k, v) > > > > as we do in tests, and the graph didn't support it, there might be a > > problem. alternatively, if a graph did support it, and the user wanted > it > > to execute as "single", then that would be a problem too. > > > > There's lots of discussion rabbit holes to go down here, but ultimately I > > think that we can honor the previous vote and support what we want if we > > implement a suggestion from Marko: > > > > public default Property property(k,v) { > > return supportsMultiProperties ? property(list, k, v) : > property(single, > > k, v) > > } > > > > then internally (within gremlin-core) we always use property(k,v) to set > a > > property (tests will continue to use explicit cardinality since that's > > already working with feature checks). Vendors can choose to override > this > > method as needed, with the understanding that internal calls from > > gremlin-core will use it for defaults. > > > > For the user: > > > > 1. They should consult their vendor implementation for their default > > operations in this area. > > 2. They should feature check their own code to be vendor agnostic. > > 3. They can avoid gremlin internals as needed and use supply their own > > getOrCreate functions thus allowing a finer degree of control (e.g. maybe > > one property key is to be loaded as list but all others should be single. > > > > If anyone has any thoughts or better ways to settle this, please let me > > know. > > > > Thanks, > > > > Stephen > > >
