ndarray dunders / binary ufuncs #17
Conversation
They will go at some point anyway.
ev-br
force-pushed
the
ndarray_dunders_ufuncs
branch
from
71bb501 to
1acb5aa
Compare
ev-br
force-pushed
the
ndarray_dunders_ufuncs
branch
from
69bfb8e to
e96e08b
Compare
ev-br
force-pushed
the
ndarray_dunders_ufuncs
branch
from
e96e08b to
d92b127
Compare
|
OK, I think this is it for now, this three-PR "stack" no longer WIP. |
|
|
||
| def __gt__(self, other): | ||
| return asarray(self._tensor > asarray(other).get()) | ||
| return _ufunc_impl.greater(self, asarray(other)) |
There was a problem hiding this comment.
Perhaps simpler to just call the functions in _binary_ufuncs? This way you could simply do
import _binary_ufuncs as bfun
__gt__ = bfun.greater
...There was a problem hiding this comment.
With a one-line helper function you could also golf this way all the in-place and reverse dunder methods.
There was a problem hiding this comment.
Sadly, _binary_ufuncs has from _ndarray import asarray, so it's an import loop.
There was a problem hiding this comment.
There has to be a way to untangle this. A (dirty) way of working around this for now would be to do a local import of the module within the class. But more generally, perhaps @rgommers has some ideas on how to work around these issues.
There was a problem hiding this comment.
Yes, I nearly did a local import; however this feels almost the same level of dirty, and I'd rather hammer it until it works with a reasonable coverage, and think through a general refactor when it does. A refactor is definitely in order (together with asarray/_from_tensor_and_base rationalization we discussed in a parallel thread, a reasonable solution to asarray imports in dtypes.py etc etc)
There was a problem hiding this comment.
So, if I understand the big picture correctly, the main issue is:
asarrayuses thendarrayconstructor- ndarray uses functions and uses
asarrayinternally in its methods - those functions use
asarrayinternally as well
Without trying to grasp the full detail here, and assuming that the solution to the circular import thing is not to throw everything into a single huge .py file, I suspect that the main problem is a lack of separation between public and private APIs. For the functions containing the main logic (item 3 above), if they were private and could assume that all inputs were already all ndarray (or maybe better, all torch.Tensor), they wouldn't have to use asarray. The public API would then be a very thin layer with the correct signatures, wrap/unwrap of input arguments and return values, and forwarding everything else to the private API. The private API would never call back to the public API layer.
|
Thanks for the reviews and discussions! Am merging this to make iterating on private/public split easier. |
This is a third PR in a "stack" of gh-12 -> gh-16 -> this one.