BUG: Allow pickling all relevant DType types/classes#18332
BUG: Allow pickling all relevant DType types/classes#18332charris merged 1 commit intonumpy:masterfrom
Conversation
numpy/core/__init__.py
Outdated
There was a problem hiding this comment.
PEP 8 wants two blank line between top level function definitions.
There was a problem hiding this comment.
Fixed, had copied it from above, but rather changed it there (and moved the copyreg to be in one spot).
ed17fd4 to
dea777d
Compare
numpy/core/__init__.py
Outdated
There was a problem hiding this comment.
There was a subtlety/issue here. the change would have made np.dtype pickle in a way that older NumPy versions cannot unpickle. But when we pickle np.dtype("f") for example, we also pickle np.dtype to unpickle it.
That would have meant that any pickle containing a dtype instance (not class) would not have been unpickable on older NumPy versions. For classes we cannot avoid that, but I am pretty sure that it would be inconvenient for the instances.
(EDIT: To be clear, in the initial version, I had return None and then special cased that in the reconstruct function.)
Introducing the metaclass without a canonical top-level name broke pickling of `type(np.dtype(...))` (which was always just `np.dtype`). While a better solution will likely be possible by making the DTypes HeapTypes and this solution may not work for all imaginable cases (i.e. it is plausible for a dtype to not have a scalar type associated), using a `copyreg` registration for the metaclass surprisingly works without any issues and seems like the simplest solution right now. Closes numpygh-16692, numpygh-18325
|
@seberg are you happy with this now? |
|
Yes, I think it should be good; if someone else with a pickle mind has a quick look, that would be great, though :). |
|
Thanks all! 😀 |
Introducing the metaclass without a canonical top-level name broke
pickling of
type(np.dtype(...))(which was always justnp.dtype).While a better solution will likely be possible by making the
DTypes HeapTypes and this solution may not work for all imaginable
cases (i.e. it is plausible for a dtype to not have a scalar type
associated), using a
copyregregistration for the metaclasssurprisingly works without any issues and seems like the simplest
solution right now.
Closes gh-16692, gh-18325