Skip to content

DEP: deprecate asscalar - #12123

Merged
charris merged 2 commits into
numpy:masterfrom
mattip:deprecate-asscalar
Oct 10, 2018
Merged

DEP: deprecate asscalar#12123
charris merged 2 commits into
numpy:masterfrom
mattip:deprecate-asscalar

Conversation

@mattip

@mattip mattip commented Oct 9, 2018

Copy link
Copy Markdown
Member

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

shoyer commented Oct 9, 2018

Copy link
Copy Markdown
Member

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

@charris

charris commented Oct 9, 2018

Copy link
Copy Markdown
Member

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

@rgommers

rgommers commented Oct 10, 2018

Copy link
Copy Markdown
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

mattip commented Oct 10, 2018

Copy link
Copy Markdown
Member Author

@charris thanks. Added actual deprecation warning.

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

@eric-wieser eric-wieser Oct 10, 2018

Copy link
Copy Markdown
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"

@mattip mattip Oct 10, 2018

Copy link
Copy Markdown
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
Comment thread numpy/lib/type_check.py Outdated

@eric-wieser eric-wieser Oct 10, 2018

Copy link
Copy Markdown
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

@mattip mattip Oct 10, 2018

Copy link
Copy Markdown
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
Comment thread numpy/lib/type_check.py Outdated
"""

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

@charris charris Oct 10, 2018

Copy link
Copy Markdown
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.

@mattip mattip Oct 10, 2018

Copy link
Copy Markdown
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

charris commented Oct 10, 2018

Copy link
Copy Markdown
Member

Thanks Matti.

@ksunden ksunden mentioned this pull request Oct 10, 2018
Merged
6 tasks done
@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

the-admax commented Apr 9, 2020

Copy link
Copy Markdown

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

eric-wieser commented Apr 9, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

@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