TYP: Fix flatiter.__next__ return type for object_ and StringDType#31660
TYP: Fix flatiter.__next__ return type for object_ and StringDType#31660jorenham wants to merge 1 commit into
flatiter.__next__ return type for object_ and StringDType#31660Conversation
| self: flatiter[NDArray[ScalarT]], / | ||
| ) -> ScalarT: ... | ||
| @overload # `StringDType` has no scalar type | ||
| def __next__(self: flatiter[np.ndarray[Any, np.dtypes.StringDType]], /) -> str: ... |
There was a problem hiding this comment.
Do you want to care about the fact that accessing a StringDType array can return the na_object as well?
>>> import numpy as np
>>> flt = np.array(["x", None], dtype=np.dtypes.StringDType(na_object=None)).flat
>>> flt[0]
'x'
>>> flt[1]
>>> flt[1] is None
True
It looks like this pattern occurs elsewhere (ndarray.item, ndarray.__iter__, ndarray.tolist, and flatiter.__getitem__(int) to name a few) so feel free to disregard this for now and do a fix focused on that later.
There was a problem hiding this comment.
Do you want to care about the fact that accessing a StringDType array can return the
na_objectas well?
Part of me does, but I'm also a bit worried that doing so will set a precedent where we'll have to add special-casing for this all over the place. So I'm not sure if that's the way we want to go, given that the stubs are already a big mess.
But on the other hand, the stubs are already a big mess, so the ∂mess/∂code probably won't be very high.
In the stubs StringDType is already generic on the na object type, so at least that part is already possible:
Line 585 in 8906919
PR summary
While working on librosa/librosa#2052, a new mypy error popped up, complaining that the iterated values of
ndarray.flatfor anobject_array aren't subscriptable, becausenp.object_does not support__getitem__. In reality,np.object_values are pure fiction, so itflatiter.__next__should instead returnAnyforobject_arrays.This also special-cases
flatiter.__next__forStringDTypearrays, which have no numpy scalar type, and would yieldbuiltins.strinstead.AI Disclosure
No AI