Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEP: Expire deprecation of dtype/signature allowing instances #22540

Merged
merged 4 commits into from Nov 9, 2022

Conversation

seberg
Copy link
Member

@seberg seberg commented Nov 7, 2022

We never really allowed instances here and deprecated it since NumPy 1.21 (it just failed completely in 1.21.0).

We never really allowed instances here and deprecated it since
NumPy 1.21 (it just failed completely in 1.21.0).
@@ -0,0 +1,3 @@
* Passing dtype instances other than the default ones to
``dtype=`` or ``signature=` in ufuncs will now raise a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"default" could use some clarification. I assume some of the outdate alternate (example?) types are deprecated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, tried to make it more specific. Basically, I still allow the singleton instances (and things that are equivalent to it). But really, we only enforce the DType (class/type) anyway, and are just generous about allowing those.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW the new wording LGTM and matches the terminology (i.e. "canonical") in the relevant NEPs

We still allow the "singleton" instances (which mainly applies to
our own dtypes), mainly because it wouldn't really do much good to
disallow them, even if they are not specific (since we don't enforce
the byte-order, but we never return non-native byte order for example).
Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo an rst formatting nit in the release note. Thanks @seberg !

doc/release/upcoming_changes/22540.expired.rst Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
* Passing dtype instances other than the default ones to
``dtype=`` or ``signature=` in ufuncs will now raise a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW the new wording LGTM and matches the terminology (i.e. "canonical") in the relevant NEPs

Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
@charris charris merged commit 2ce5416 into numpy:main Nov 9, 2022
@charris
Copy link
Member

charris commented Nov 9, 2022

Thanks Sebastian,

@seberg seberg deleted the finalize-dtype-depr branch November 9, 2022 16:12
@gmarkall gmarkall mentioned this pull request Nov 24, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants