-
Notifications
You must be signed in to change notification settings - Fork 905
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
[Fix] Fix numpy.core.einsumfunc deprecation #628
Conversation
And this might actually break autograd in combination with numpy<2 so we would either need to bump the dependency version or change this to a version dependent import. Any preference @agriyakhetarpal ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for noting that, @fjosw! Yes, it might be too early to remove backwards compatibility with NumPy <2, so a version-dependent import would be better here. I think the best time to enforce NumPy>2 support would be when SciPy itself can't build against NumPy<2 – newer releases for it can do so right now just fine, so that day would be the one to suggest that NumPy v1 can be considered legacy code (or when the majority of Cython extensions will build against NumPy 2, which would take longer).
Also, we do have some bits of legacy code here and there, plus imports of private modules like this – and it will become difficult to fix them in the coming time, so we need a plan. I would propose running tests with the NumPy and SciPy nightlies so that we can catch such deprecations/removals early, which I can volunteer to implement once I migrate the tox
testing configuration to nox
:)
Also, I think it would make sense to run tests against a suitable NumPy v1.X to determine and subsequently maintain a lower bound for NumPy versions going forward, do you have any thoughts?
See also:
|
I agree with you, that we should probably keep NumPy 1 compatibilty for now (even though people could just use an older version for now, at least until we do not add additional features). And thanks for offering to include tests with the Nightly builds! And I also agree, that we should run at least one ci workflow with NumPy 1 as long as we want to support it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your thoughts! I'll work on those soon, then. Please feel free to merge if this is ready, or fix any other similar deprecation warnings if there are more.
After the 1.7.0 release I noticed the following deprecation warning:
This PR simply replaces the import from
numpy.core.einsumfunc
bynumpy._core.einsumfunc
which should fix the warning but adds the risk that_parse_einsum_input
might break in a furture numpy release without prior warning.