Thanks, Matthias....I should have included the key in what i was proposing. In that way, we could explicitly do:
property(vertexFeatures.getCardinality(k),k,v) in the attachment process or anywhere else that this mattered. On Mon, May 4, 2015 at 3:51 PM, Matthias Broecheler <[email protected]> wrote: > Hi Stephen, > > so, here is where the default method is really useful imho: for graphs with > schema. TinkerGraph implicitly has the schema that all keys are > multi-valued. Other graphs might choose a different default. > Titan (and other graphs with customizable schema) can use this method to > automatically infer the right cardinality. > In Titan, users define their keys and specify the cardinality in that > definition. It would be extremely cumbersome to require the user to > redundantly specify that cardinality every time they use the key. > > So, for instance, if the key "name" is defined with single cardinality, > then it is clear that property("name","Stephen") implies > cardinality=single. > I am fine with having a feature method to gives you the default > cardinality, but it would have to be on a per key basis: > Graph.features().vertex().getDefaultCardinality(String key) > > Thanks, > Matthias > > > On Mon, May 4, 2015 at 4:03 AM Stephen Mallette <[email protected]> > wrote: > > > 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 > > > > > > > > > >
