Skip to content

ENH: set correct __module__ for objects in numpy's public API #12382

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

Merged
merged 6 commits into from
Nov 15, 2018

Conversation

shoyer
Copy link
Member

@shoyer shoyer commented Nov 14, 2018

Fixes #12271

Tests verify that everything in dir(numpy) either has __module__ set to 'numpy', or appears in an explicit whitelist of undocumented functions and exported bulitins. These should eventually be documented or removed.

I also identified a handful of functions for which I had accidentally not setup dispatch for with __array_function__ before, mostly because they were listed under "ndarray methods" in _add_newdocs.py. I guess that should be a lesson in trusting code comments :).

Fixes numpyGH-12271

Tests verify that everything in ``dir(numpy)`` either has ``__module__`` set to
``'numpy'``, or appears in an explicit whitelist of undocumented functions and
exported bulitins. These should eventually be documented or removed.

I also identified a handful of functions for which I had accidentally not setup
dispatch for with ``__array_function__`` before, because they were listed under
"ndarray methods" in ``_add_newdocs.py``. I guess that should be a lesson in
trusting code comments :).
'set_string_function': 'numpy.core.arrayprint.set_string_function',
'show_config': 'numpy.__config__.show',
'str': 'builtins.str',
'unicode': 'builtins.str',
Copy link
Member

Choose a reason for hiding this comment

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

Can you group together the builtins aliases, perhaps even as a separate dictionary?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@charris
Copy link
Member

charris commented Nov 14, 2018

The downside is that it is harder to find the actual location. Oh, well, there is still grep. Seems like something that could use a mention in the release notes, but I can't figure where :) Maybe compatibility notes?

@charris charris added this to the 1.16.0 release milestone Nov 14, 2018
@shoyer
Copy link
Member Author

shoyer commented Nov 14, 2018

I actually already wrote release notes for this as part of one of my __array_function__ PRs: https://github.com/numpy/numpy/blob/2668b3131812ffd87be335803ca6bb8160234424/doc/release/1.16.0-notes.rst#__module__-attribute-now-points-to-public-modules (this is on numpy master)

@charris charris merged commit d1e0a43 into numpy:master Nov 15, 2018
@charris
Copy link
Member

charris commented Nov 15, 2018

OK, in it goes. Thanks Stephan.

@jnothman
Copy link
Member

This appears to have broken PyPy support (#12740), since PyPy treats __module__ as read-only.

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.

4 participants