DEP: Correct dtype in __array_finalize__ if _set_dtype=None - #31293
Conversation
This avoids using the correct dtype in __array_finalize__ *unless* `_set_dtype = None` is set on the class. We also use this new path to make record arrays follow the main class deprecation correctly. This means that `recarray` does live in the future.
|
|
||
| def __array_finalize__(self, obj): | ||
| if self.dtype.type is not record and self.dtype.names is not None: | ||
| if issubclass(self.dtype.type, nt.void) and self.dtype.names is not None: |
There was a problem hiding this comment.
This is the logic used in __setattr__, basically the same I guess, but thought I'd just copy paste it.
There was a problem hiding this comment.
EDIT: Actually, no, I'll just undo this to avoid creating a new dtype here if it isn't necessary. After that dtype.type is always record, so that check should be enough.
Made it to have both checks. The second one is needed for some ridiculous tests to pass, but the original one is definitely OK as a fast-path.
ddec556 to
6895cbc
Compare
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
This does seem a good path forward! My comments are minimal, mostly about comments, so will approve now.
p.s. I may still follow up with changing the dims & strides beforehand.
| * Set ``_set_dtype = None`` in which case ``arr.view(dtype=new_dtype)`` | ||
| will call ``__array_finalize__`` with the new dtype, ensuring that | ||
| any validation ``__array_finalize__`` will run is done. | ||
| * Define ``_set_dtype`` as a function (calling ``ndarray._set_dtype()`` |
There was a problem hiding this comment.
How about, "Of, for a quick fix to avoid DeprecationWarning, define ..."
| * setter): create subclass view first, use the setter, but | ||
| * emit a deprecation asking to implement _set_dtype instead. | ||
| * 3. Otherwise (including plain ndarray): create an ndarray base | ||
| * 3. If _set_dtype is None (or base-class): create an ndarray base |
There was a problem hiding this comment.
I think "or base-class" is incorrect, otherwise how would one ever get to path 4?
| * 3. If _set_dtype is None (or base-class): create an ndarray base | ||
| * view, set dtype internally, then create the subclass view | ||
| * if needed. __array_finalize__ sees the final dtype+shape. | ||
| * 4. Otherwise, call `__array_finalize__` with old dtype and forcibly |
There was a problem hiding this comment.
Maybe you meant to write it here, like If _set_dtype is NULL or base-class: ...
| * with correct dtype+shape, this will call `__array_finalize__` | ||
| * with the final dtype+shape. | ||
| */ | ||
| if (use_dtype_in_finalize && subtype != &PyArray_Type) { |
There was a problem hiding this comment.
use_dtype_in_finalize can only be set if it is a subclass, so second part can be omitted.
…31293) This avoids using the correct dtype in array_finalize unless _set_dtype = None is set on the class. We also use this new path to make record arrays follow the main class deprecation correctly. This means that recarray does live in the future.
3 participants
This avoids using the correct dtype in array_finalize unless
_set_dtype = Noneis set on the class.We also use this new path to make record arrays follow the main class deprecation correctly.
This means that
recarraydoes live in the future.CC @mhvk, hopefully the last iteration (unless the larger
viewchange works out)...(no AI tool use)
Closes gh-31281