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
> > >
> >
>
