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

Fix numpy 1.25 deprecation warnings. #14510

Merged
merged 4 commits into from
Mar 12, 2023
Merged

Fix numpy 1.25 deprecation warnings. #14510

merged 4 commits into from
Mar 12, 2023

Conversation

dstansby
Copy link
Contributor

@dstansby dstansby commented Mar 9, 2023

In numpy 1.25 several functions will be deprecated. I've either replaced the functions with like-for-like functions that aren't deprecated, or filtered the warnings in tests that should stay.

Fixes #14509.

@github-actions
Copy link

github-actions bot commented Mar 9, 2023

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

Copy link
Member

@larrybradley larrybradley left a comment

Choose a reason for hiding this comment

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

convolution changes look fine.

@dstansby dstansby requested a review from saimn as a code owner March 9, 2023 16:34
@dstansby dstansby changed the title Use np.prod instead of np.product Fix numpy 1.25 deprecation warnings. Mar 9, 2023
@dstansby
Copy link
Contributor Author

dstansby commented Mar 9, 2023

While I was at it I updated this to hopefully fix all the numpy 1.25 deprecation warnings

@pllim pllim added this to the v5.0.6 milestone Mar 9, 2023
@pllim pllim added numpy-dev no-changelog-entry-needed 💤 backport-v5.0.x on-merge: backport to v5.0.x 💤 backport-v5.2.x on-merge: backport to v5.2.x labels Mar 9, 2023
@pllim
Copy link
Member

pllim commented Mar 9, 2023

@dstansby
Copy link
Contributor Author

dstansby commented Mar 9, 2023

Also, I think you missed one -- https://github.com/astropy/astropy/actions/runs/4376653449/jobs/7659080296

It looks like that's coming from asdf, not astropy

@pllim
Copy link
Member

pllim commented Mar 9, 2023

In that case, please ignore that warning locally for this failure. Thanks!

        if Version(asdf.__version__) >= Version("2.8.0"):
            # The auto_inline argument is deprecated as of asdf 2.8.0.
            with asdf.config_context() as config:
                config.array_inline_threshold = 64
>               assert_roundtrip_tree({"table": t}, tmpdir, asdf_check_func=check)

astropy/io/misc/asdf/tags/table/tests/test_table.py:126: 

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

This looks good! I put an inline note that we did it a bit differently in #14484, so maybe we should keep it the same. But really it doesn't matter; could even change the construct there to be the same as you have it here.

@pllim
Copy link
Member

pllim commented Mar 9, 2023

👍 to keep things consistent with #14484

@dstansby
Copy link
Contributor Author

👍 I went with the option of filtering the warnings, as it leads to a much clearer test function body.

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Looks all good to me!

@@ -634,10 +634,12 @@ def test_all(self):
with pytest.raises(TypeError):
np.all(self.q)

@pytest.mark.filterwarnings("ignore:`sometrue` is deprecated as of NumPy 1.25.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 don't like removing NUMPY_LT_1_25. Makes it hard to grep and remove dead code when we eventually bump minversion to numpy 1.25. Can we at least leave # NUMPY_LT_1_25 here for future grep?

Copy link
Member

Choose a reason for hiding this comment

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

Unless numpy dev remove it first before that happens but I can't tell the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 done

Copy link
Contributor

@WilliamJamieson WilliamJamieson left a comment

Choose a reason for hiding this comment

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

Modeling changes look fine to me

@mhvk
Copy link
Contributor

mhvk commented Mar 12, 2023

@dstansby - thanks! let's get this in.

@mhvk mhvk merged commit 64f58ae into astropy:main Mar 12, 2023
@lumberbot-app
Copy link

lumberbot-app bot commented Mar 12, 2023

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout v5.0.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 64f58aeac6a8651a5e7c405bbbab837f73d89fb2
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #14510: Fix numpy 1.25 deprecation warnings.'
  1. Push to a named branch:
git push YOURFORK v5.0.x:auto-backport-of-pr-14510-on-v5.0.x
  1. Create a PR against branch v5.0.x, I would have named this PR:

"Backport PR #14510 on branch v5.0.x (Fix numpy 1.25 deprecation warnings.)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

@lumberbot-app
Copy link

lumberbot-app bot commented Mar 12, 2023

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout v5.2.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 64f58aeac6a8651a5e7c405bbbab837f73d89fb2
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #14510: Fix numpy 1.25 deprecation warnings.'
  1. Push to a named branch:
git push YOURFORK v5.2.x:auto-backport-of-pr-14510-on-v5.2.x
  1. Create a PR against branch v5.2.x, I would have named this PR:

"Backport PR #14510 on branch v5.2.x (Fix numpy 1.25 deprecation warnings.)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

mhvk added a commit to mhvk/astropy that referenced this pull request Mar 12, 2023
Fix numpy 1.25 deprecation warnings.

(cherry picked from commit 64f58ae)
mhvk added a commit to mhvk/astropy that referenced this pull request Mar 12, 2023
Fix numpy 1.25 deprecation warnings.

(cherry picked from commit 64f58ae)
mhvk added a commit that referenced this pull request Mar 12, 2023
Backport PR #14510 on branch v5.2.x (Fix numpy 1.25 deprecation warnings.)
mhvk added a commit that referenced this pull request Mar 12, 2023
Backport PR #14510 on branch v5.0.x (Fix numpy 1.25 deprecation warnings.)
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.

numpy DeprecationWarning emitted due to use of np.product
5 participants