Skip to content

ENH: provide a convenience function to replace npy_load_module#20395

Merged
WarrenWeckesser merged 3 commits intonumpy:mainfrom
mattip:exec_module
Nov 19, 2021
Merged

ENH: provide a convenience function to replace npy_load_module#20395
WarrenWeckesser merged 3 commits intonumpy:mainfrom
mattip:exec_module

Conversation

@mattip
Copy link
Member

@mattip mattip commented Nov 17, 2021

load_module is deprecated since python 3.4 and will be removed in python 3.12. Use exec_module instead. Provide a convenience function in distutils.misc_utils instead of npy_load_module from compat.py3k.

This PR was triggered by deprecation warnings in python 3.10.

@mattip
Copy link
Member Author

mattip commented Nov 17, 2021

This fixes the failing conda builds

Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Generally, looks good to me, but I don't know this well (would be nice to get rid of the CI failure though).

One reference I found: #17731 (comment) there is a comment that we have to add the new module to sys.modules? And that snippet infers the name from the filename, although I am not sure that is actually what we do here.


atexit.register(clean_up_temporary_directory)

from numpy.compat import npy_load_module
Copy link
Member

Choose a reason for hiding this comment

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

Should we leave the import here since this is probably somewhat public?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a compatibility shim. I think not.

Copy link
Member Author

Choose a reason for hiding this comment

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

From perusing the first couple of pages at searchcode, it seems users are properly importing it from numpy.compat and not numpy.distutils.misc_utils.

@mattip
Copy link
Member Author

mattip commented Nov 18, 2021

I added a comment about sys.modules to the function docstring. I prefer to minimally modify global state in the function itself. The function is for once-off loading, and if desired the caller can add the module to sys.modules.

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Nov 18, 2021
@charris charris added this to the 1.22.0 release milestone Nov 18, 2021
@mattip mattip requested a review from rgommers November 19, 2021 10:54
@mattip
Copy link
Member Author

mattip commented Nov 19, 2021

Maybe we can put this in as-is and iterate on it as needed. The CI failures are annoying.

Copy link
Member

@WarrenWeckesser WarrenWeckesser left a comment

Choose a reason for hiding this comment

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

Looks good. I'll fix one small flake8 issue and merge.

@WarrenWeckesser WarrenWeckesser merged commit 46ec71f into numpy:main Nov 19, 2021
charris pushed a commit to charris/numpy that referenced this pull request Nov 19, 2021
…#20395)

`load_module` is deprecated since python 3.4 and will be removed in python 3.12.
Use `exec_module` instead. Provide a convenience function in `distutils.misc_utils`
instead of `npy_load_module` from `compat.py3k`.
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Nov 19, 2021
@charris charris removed this from the 1.22.0 release milestone Nov 19, 2021
@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Nov 19, 2021
@charris charris added this to the 1.21.5 release milestone Nov 19, 2021
charris added a commit that referenced this pull request Nov 19, 2021
ENH: provide a convenience function to replace npy_load_module (#20395)
charris pushed a commit to charris/numpy that referenced this pull request Nov 25, 2021
…#20395)

`load_module` is deprecated since python 3.4 and will be removed in python 3.12.
Use `exec_module` instead. Provide a convenience function in `distutils.misc_utils`
instead of `npy_load_module` from `compat.py3k`.
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Nov 25, 2021
@charris charris removed this from the 1.21.5 release milestone Nov 25, 2021
charris pushed a commit to charris/numpy that referenced this pull request Nov 26, 2021
…#20395)

`load_module` is deprecated since python 3.4 and will be removed in python 3.12.
Use `exec_module` instead. Provide a convenience function in `distutils.misc_utils`
instead of `npy_load_module` from `compat.py3k`.
@mattip mattip deleted the exec_module branch December 27, 2022 16:49
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