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

DEP: Deprecate 'fix_imports' flag in numpy.save #26452

Merged
merged 10 commits into from
May 17, 2024

Conversation

vxst
Copy link
Contributor

@vxst vxst commented May 16, 2024

Since NumPy upgraded the pickle protocol version to 3 in version 1.17, the fix_imports flag is ignored by the pickle library. To reduce confusion, I propose removing this flag in a future release.

However, since many people actually use numpy.save(..., allow_pickle=True, fix_imports=True) as default behavior (for example, see this example: intel/intel-extension-for-transformers), removing the fix_imports keyword may break compatibility with existing code.

Following NEP 23, I suggest adding a check for the usage of the fix_imports keyword and emitting a DeprecationWarning for any usage.

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.

Thanks a lot, always good to get a deprecation started!

For Python deprecations it is a bit less of a worry, but we should add a short test to the numpy/_core//tests/test_deprecations.py file.

will try to map the new Python 3 names to the old module names used in
Python 2, so that the pickle data stream is readable with Python 2.
The `fix_imports` flag is deprecated and has no effect.
.. deprecated:: 2.0
Copy link
Member

Choose a reason for hiding this comment

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

I would lean towards just not backporting, doesn't seem much of a deal. @charris want to make the call?

Historically, this flag was used to control compatibility support
for objects saved in Python 3 to be loadable in Python 2. This flag
is ignored after NumPy 1.17, and deprecated in NumPy 2.0. It will
be removed in a future release.
Copy link
Member

Choose a reason for hiding this comment

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

Looks good as is, might suggest to shorten it a bit: This flag is ignored since NumPy 1.17 and was only needed to support loading some files in Python 2 written in Python 3.

numpy/lib/_npyio_impl.py Show resolved Hide resolved
numpy/lib/_npyio_impl.py Outdated Show resolved Hide resolved
@ngoldbaum ngoldbaum self-assigned this May 17, 2024
@ngoldbaum
Copy link
Member

Thanks @vxst!

@ngoldbaum
Copy link
Member

Was about to merge and then I realized that this needs a release note too. Can you add one?

@ngoldbaum ngoldbaum added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label May 17, 2024
@vxst
Copy link
Contributor Author

vxst commented May 17, 2024

Of course

@vxst
Copy link
Contributor Author

vxst commented May 17, 2024

Sorry that I forgot deprecation needs a release note.

@ngoldbaum ngoldbaum merged commit 67257a7 into numpy:main May 17, 2024
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
07 - Deprecation 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants