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

numpy deprecation warning filter for v1.24.0 #1873

Merged
merged 13 commits into from
Nov 29, 2022

Conversation

bmorris3
Copy link
Contributor

Numpy v1.24.0 triggers several deprecation warnings in scikit-image. See progress on fixes in scikit-image/scikit-image#6633, and failure in #1834.

This PR tries to filter out that specific deprecation warning.

Description

This pull request is to address ...

Fixes #

Change log entry

  • Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
    list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
    should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
  • Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
  • Do the proposed changes follow the STScI Style Guides?
  • Are tests added/updated as required? If so, do they follow the STScI Style Guides?
  • Are docs added/updated as required? If so, do they follow the STScI Style Guides?
  • Did the CI pass? If not, are the failures related?
  • Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
  • After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

@bmorris3 bmorris3 added this to the 3.1.2 milestone Nov 28, 2022
@bmorris3 bmorris3 added trivial Only needs one approval instead of two no-changelog-entry-needed changelog bot directive Affects-dev changelog bot directive labels Nov 28, 2022
@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Base: 88.37% // Head: 88.37% // No change to project coverage 👍

Coverage data is based on head (c12b10a) compared to base (bcc29e7).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1873   +/-   ##
=======================================
  Coverage   88.37%   88.37%           
=======================================
  Files          95       95           
  Lines       10456    10456           
=======================================
  Hits         9241     9241           
  Misses       1215     1215           
Impacted Files Coverage Δ
...onfigs/mosviz/plugins/slit_overlay/slit_overlay.py 71.05% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Blanket ignore seems overkill for something that only crops up on dev job.

Lemme catch up on the logs and see if we can do something more localized.

@bmorris3
Copy link
Contributor Author

bmorris3 commented Nov 28, 2022

@pllim – to help you up to speed, see this warning, for example.

Would you be more comfortable with a more specific ignore for np.bool8 is a deprecated alias for np.bool_.?

@pllim
Copy link
Contributor

pllim commented Nov 28, 2022

Argh, so it is from glue_jupyter/bqplot/image/layer_artist.py import. That is unfortunately. Yes, a more specific ignore as you suggested would be safer, so we do not accidentally ignore other deprecation warning from numpy. Thanks!

@pllim
Copy link
Contributor

pllim commented Nov 28, 2022

You might need to fix the regex.

@bmorris3
Copy link
Contributor Author

@pllim That's what I was worried about, but couldn't figure out how to test it locally. Trying again.

setup.cfg Outdated Show resolved Hide resolved
bmorris3 and others added 2 commits November 28, 2022 14:48
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
setup.cfg Outdated Show resolved Hide resolved
@pllim
Copy link
Contributor

pllim commented Nov 28, 2022

Maybe it was int0 and not uint0 or both?

DeprecationWarning: `np.int0` is a deprecated alias for `np.intp`.

@pllim
Copy link
Contributor

pllim commented Nov 28, 2022

Also these are strictly upstream right? Do you plan to fix uint0 and int0 upstream too?

@pllim pllim added the 💤backport-v3.1.x on-merge: backport to v3.1.x label Nov 29, 2022
Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

When CI pass, I will squash and merge.

Note to self: Open follow up issue to undo this.

@pllim pllim modified the milestones: 3.1.2, 3.2 Nov 29, 2022
@pllim pllim removed the 💤backport-v3.1.x on-merge: backport to v3.1.x label Nov 29, 2022
@pllim

This comment was marked as resolved.

@pllim
Copy link
Contributor

pllim commented Nov 29, 2022

Remaining failures in the devdeps job all use https://stsci.box.com/shared/static/exnkul627fcuhy5akf2gswytud5tazmw.fits . Curious that this file worked with the other remote data job but not this one, but investigating it is out of scope here.

Update: Maybe this file is too old and not compatible with latest asdf stuff. Should replace it with real data from orbit.

@pllim pllim merged commit 0b76ee4 into spacetelescope:main Nov 29, 2022
pllim added a commit that referenced this pull request Nov 29, 2022
Co-authored-by: Brett M. Morris <bmmorris@stsci.edu>
ra = np.array(footprint[::2], dtype=np.float)
dec = np.array(footprint[1::2], dtype=np.float)
ra = np.array(footprint[::2], dtype=float)
dec = np.array(footprint[1::2], dtype=float)
Copy link
Contributor

Choose a reason for hiding this comment

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

I backported this to v3.1.x in d7ec0ab

pllim added a commit to meeseeksmachine/jdaviz that referenced this pull request Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects-dev changelog bot directive no-changelog-entry-needed changelog bot directive trivial Only needs one approval instead of two
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants