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

ENH: Added sanity check to printoptions #18263

Merged

Conversation

Mitchell-Faas
Copy link
Contributor

See issue #18254

@rgommers
Copy link
Member

Restarting CI, something weird happened.


if precision is not None:
# forbid the bad precision arg as suggested by issue #18254
if not isinstance(precision, int):
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps better as precision = operator.index(precision), else this blows up if users pass an np.int32 or something else that's harmless

Fixed a bug where precision couldn't be a non-native integer.
See comments on pull request numpy#18263 for more info.
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

I think we normally don't do from operator import *, as those names are very likely to overlap with other names. If nothing else, it makes operator.index something which can be searched for.

numpy/core/arrayprint.py Outdated Show resolved Hide resolved
numpy/core/arrayprint.py Outdated Show resolved Hide resolved
@BvB93
Copy link
Member

BvB93 commented Jan 31, 2021

Can you update the annotations in arrayprint.pyi as well?

In total two changes will be necessary:

  1. Import the SupportsIndex protocol right here:
if sys.version_info > (3, 8):
    from typing import SupportsIndex
else:
    from typing_extensions import SupportsIndex
  1. Replace int with SupportsIndex (for the precision parameter) in the following functions:

Copy link
Member

@BvB93 BvB93 left a comment

Choose a reason for hiding this comment

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

The annotation update is looking good as well.

@charris charris merged commit 2d6c555 into numpy:master Feb 1, 2021
@charris
Copy link
Member

charris commented Feb 1, 2021

Thanks @Mitchell-Faas .

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

5 participants