Skip to content

DEP: deprecate asscalar #12123

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 10, 2018
Merged

DEP: deprecate asscalar #12123

merged 2 commits into from
Oct 10, 2018

Conversation

mattip
Copy link
Member

@mattip mattip commented Oct 9, 2018

Closes #4701 (issue). Closes #11856 (PR). Closes #10256 (PR). Closes #10659 (PR).

This issue was labelled "easy" which led to the multiple PRs to fix it. I propose to deprecate it instead, as it is neither used nor tested in numpy, and is a thin wrapper to ndarray.item() but accepts none of the args.

SciPy does not use asscalar, astropy has two uses of asscalar which would need fixing.

@shoyer
Copy link
Member

shoyer commented Oct 9, 2018

👍 This seems like a nice resolution to me. The function is basically useless.

@charris
Copy link
Member

charris commented Oct 9, 2018

Hmm, what is the reason to only deprecate it in the docstring? Should it raise a deprecation warning?

@rgommers
Copy link
Member

Agree that it needs a DeprecationWarning. Did a quick search - zero usage in scipy, scikit-image and statsmodels, 1 occurrence in scikit-learn. Won't impact users much.

@mattip mattip force-pushed the deprecate-asscalar branch from 1c2a643 to af5d32c Compare October 10, 2018 05:04
@mattip
Copy link
Member Author

mattip commented Oct 10, 2018

@charris thanks. Added actual deprecation warning.

eric-wieser
eric-wieser reviewed Oct 10, 2018
View reviewed changes
numpy/lib/type_check.py Outdated

# 2018-10-10, 1.16
warnings.warn('asscalar will be removed in v1.18 of numpy',
VisibleDeprecationWarning)
Copy link
Member

Choose a reason for hiding this comment

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

No need for VisibleDeprecationWarning I think - use the normal one.

Also, this should suggest something like "use .item() instead"

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

eric-wieser
eric-wieser reviewed Oct 10, 2018
View reviewed changes
numpy/lib/type_check.py Outdated
@@ -467,7 +467,10 @@ def real_if_close(a,tol=100):

def asscalar(a):
"""
Convert an array of size 1 to its scalar equivalent.
Convert an array of size 1 to its scalar equivalent. Deprecated, use
`numpy.ndarray.item()` instead.
Copy link
Member

Choose a reason for hiding this comment

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

This second sentence should be indented under the .. deprecated block

Copy link
Member Author

Choose a reason for hiding this comment

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

moved

@mattip mattip force-pushed the deprecate-asscalar branch from af5d32c to 1c2abd3 Compare October 10, 2018 10:02
charris
charris reviewed Oct 10, 2018
View reviewed changes
numpy/lib/type_check.py Outdated
@@ -486,6 +491,10 @@ def asscalar(a):
24

"""

# 2018-10-10, 1.16
warnings.warn('np.asscalar(a) will be removed in v1.18 of numpy, use '
Copy link
Member

Choose a reason for hiding this comment

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

I think we are headed toward using the version deprecated rather than the version in which we intend to remove it. The latter may change, the first is fixed and helps folks downstream know how to set up version dependent workarounds.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for fixing

@charris charris merged commit 3626bf4 into numpy:master Oct 10, 2018
@charris
Copy link
Member

charris commented Oct 10, 2018

Thanks Matti.

@ksunden ksunden mentioned this pull request Oct 10, 2018
Merged
6 tasks
@mattip mattip mentioned this pull request Oct 10, 2018
Closed
@mattip mattip deleted the deprecate-asscalar branch October 10, 2018 17:00
@mattip mattip mentioned this pull request Oct 10, 2018
Closed
@ksunden ksunden mentioned this pull request Oct 10, 2018
Merged
@ax3l ax3l mentioned this pull request Sep 6, 2019
Merged
@the-admax
Copy link

Sorry, but why this handy function got deprecated in favor of x.item()? I find the functional approach more robust in dynamic typing world and providing more flexibility than attribute access when type of x is not known. For instance, this is a common case to use python print or string formatting functions when a value coming from array operation is actually a deeply nested scalar like [[1]]. So what is a proper alternative for this deprecated function?

@eric-wieser
Copy link
Member

eric-wieser commented Apr 9, 2020 •
edited
Loading

@the-admax: ascalar(x) was implemented as x.item() anyway, so behaved identically.

providing more flexibility than attribute access when type of x is not known

Both would fail if x was not a numpy type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charris charris charris left review comments

@eric-wieser eric-wieser eric-wieser left review comments

Assignees
No one assigned
Labels
07 - Deprecation component: numpy._core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

np.asscalar should pass through scalars
6 participants
@mattip @shoyer @charris @rgommers @the-admax @eric-wieser