Conversation
| def __iter__(self) -> Iterator[T]: | ||
| """ | ||
| Useful element of a Sequence to include. | ||
| """ |
There was a problem hiding this comment.
@HDembinski, you've already okay'd the np.ndarray return, how about this? Are you okay with requiring any axes also be iterable? Once 3.6 is dropped we could further restrict to axes being a Sequence, or we could manually add all the parts of the Sequence protocol (there are only a handful more), but this is probably the most important part of a sequence and will simplify downstream code that currently must use __len__ and __getitem__ to be valid. I believe @jpivarski has more-or-less okay'd both. (though please speak up if not!)
There was a problem hiding this comment.
(note: the container .axes holding Axis objects is what's being discussed here, not the Axis objects themselves.)
There was a problem hiding this comment.
Sure, this makes sense. Iterable is safe. I am not sure whether we want to require full Sequence support. __reversed__ and count does not make much sense.
There was a problem hiding this comment.
(note: the container .axes holding Axis objects is what's being discussed here, not the Axis objects themselves.)
Ahh, I made a mistake. This is the Axis object. We already require the axes to be in a Sequence (which is why we require __eq__ on an Axis). I think this is still fine, but requires more thought that I first expected. I think that something that provides __getitem__ and __len__ should likely provide an __iter__; we do in boost-histogram (and it's trivial to provide if you have the other two). I caught it with the NumPyPlottibleHistogram, which is a minimal, unfillable histogram that adapts other histogram types to the PlottableHistogram protocol. It didn't have an __iter__, though I'm happy to add it. Any second thoughts, @HDembinski or @jpivarski ?
There was a problem hiding this comment.
Thanks, sorry for the confusion!
2 participants
Making this extend
Sequence[T]turns out to be rather hard in Python 3.6; due to the fact Generic types in 3.6 were actually objects and not classes, you couldn't inherit from one. In 3.7, this was fixed by the addition of__class_getitem__and a few other things, so Generics really are just normal classes (literally - in 3.9+ the built in classes now work as Generics using this). See python/typing#561. This is also a large part of why importing the typing module in 3.7+ is 7x faster, it's not making a bunch of objects anymore, just classes.I've added the one really useful "Sequence" method here; we could add the rest (in which case, this really would become a Sequence, as a Protocol is just that - no need to subclass), but for now, it seems this is close enough, I think. It will simply downstream code.
This closes #10 and the most important part of, and closes #6.