Simplify fetch

[fabiansinz]

Currently, the fetch object is not elegant and potentially confusing to users. I propose the following changes to simplify it.

Stateless fetch
fetch becomes completely stateless. This will simplify the implementation and make the process more transparent. All states become keyword arguments in fetch. For example rel.fetch.order_by('attribute').as_dict() becomes rel.fetch(as_dict=True, order_by='attribute'). All previous states become forced keyword arguments.

getitem and iterators
Fetch keeps __getitem__, keys, and __iter__. The disadvantage that comes with the stateless implementation is that the ways the elements are fetched cannot easily be modified for most of these functions. The following changes would solve this

• keys does accept arguments like __call__ (i.e. rel.keys(order_by='attr1'))

• __call__ does also accept unnamed arguments. In that case it behaves like __getitem__ but with possible modifications. For example a1, a2 = rel.fetch['attr1','attr2'] is equivalent to a1, a2 = rel.fetch('attr1','attr2'). However, __call__ can be modified such as a1, a2 = rel.fetch('attr1','attr2', order_by='attr1').

• __iter__ stays, but cannot be modified. In addition, we provide the iterator traverse which accepts the same modifying keyword arguments as __call__. Additionally, traverse can get unnamed arguments. For example for a1, a2 in rel.fetch.traverse('attr1','attr2', limit=10):.

[dimitri-yatsenko]

Yes, agreed.

[dimitri-yatsenko]

If fetch is going to be stateless, there is little reason to keep it as a class. It can become a method of RelationalOperand. traverse then will be used rather than fetch.traverse.

[fabiansinz]

I think we should keep it for two reasons:

• __getitem__ needs an object and we might want to use getitem in the relation at some point (e.g. projections).

• it keeps all elements related to fetching separated from the relational algebra.

[eywalker]

Yes let's not remove fetch object all of a sudden - we have introduced it precisely for the above two reasons that @fabiansinz pointed out. Making it stateless is a distinct consideration. We also wanted to make it obvious that no elements are fetched (except for some for the preview purpose) unless you use methods on rel.fetch.

However I'm not sure about making fetch() act essentially as a superset of fetch[]. I think I would prefer that fetch('a', 'b') to perform something different from fetch['a', 'b'] or otherwise I don't see much use of fetch[] and this can actually confuse the user.

[eywalker]

Also, recall that a lot of this stateful approach got added to support the fetch['a'] syntax as there is no way to pass in arguments like limit=10.

I agree that current stateful approach has led to a rather ugly syntax and I'd love to simplify this, but I also don't like to see us suddenly providing many different ways to do exactly the same thing - that makes thing less simple.

We also have to be quite careful about such major syntax change as this WILL break some of the current user's code (in the least due to removal of things like .fetch.order_by() but potentially more if we decide to make larger syntax changes such as removal of fetch[]). Technically datajoint-python is still in it's pre-release state (version is < 1.0.0) so some large syntax change might be expected or at least somewhat acceptable but we'd have to be very sure that this is what we want and have come out with a strategy to gracefully transition users to the new syntax.

[eywalker]

That being said, I do really like the new proposed syntax of fetch('a', 'b', order_by='a') and this can potentially bring about new call like fetch1('a', order_by='a') to mean fetch the first entry after ordering by a - this has the benefit of returning the element by itself and not in a list.

[fabiansinz]

I view the __getitem__ as pure convenience. I don't have a problem with some redudancy in how you do things. In fact, I would like to put most of the logic into __call__ and use the other methods to simplify syntax.

[eywalker]

Well my point is that if we make the suggested changes to __call__ then there is nothing convenient about fetch[] at all.

[fabiansinz]

True. So we could throw a deprecation warning for a while and then get rid of it.

[eywalker]

Yeah that might be the best move. I like the idea of centralizing most things into the __call__. We should at least update docs and tutorials to use new syntax only ASAP if we decide to make this switch.

[eywalker]

Ok I think we are converging on the decision to:

1. bring in optional unnamed arguments into __call__ method on fetch, supporting syntax like rel.fetch('a', 'b'), essentially acting equivalently to the current implementation of rel.fetch['a', 'b']
2. deprecate the use of __getitem__ on fetch (i.e. no more rel.fetch['a'] syntax)
3. make fetch completely stateless - therefore deprecate state modifiers like order_by() and limit() on fetch

I'm going to start the implementation of the above points most likely in that very order and we'll make this part of the release 0.7.0.