This: property(vertexFeatures.getCardinality(k),k,v)
is now implemented. The default is to return Cardinality.list. It is up to vendors to override that as required. On Mon, May 4, 2015 at 4:13 PM, Stephen Mallette <[email protected]> wrote: > 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 >> > > > >> > > >> > >> > >
