Skip to content

TST: Unignore cannot resolve package warning in pytest - #11928

Merged
saimn merged 2 commits into
astropy:mainfrom
pllim:tst-unignore-cannot-resolve-package
Jul 9, 2021
Merged

TST: Unignore cannot resolve package warning in pytest#11928
saimn merged 2 commits into
astropy:mainfrom
pllim:tst-unignore-cannot-resolve-package

Conversation

@pllim

@pllim pllim commented Jul 8, 2021

Copy link
Copy Markdown
Member

Description

This pull request is to address #6025 (comment)

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.

  • 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.
  • Is a change log needed? If yes, did the change log check pass? If no, add the no-changelog-entry-needed label.
  • Is a milestone set? Milestone must be set but astropy-bot check might be missing; 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.

@pllim pllim added no-changelog-entry-needed Extra CI Run cron CI as part of PR labels Jul 8, 2021
@pllim pllim added this to the v5.0 milestone Jul 8, 2021
@pllim pllim mentioned this pull request Jul 8, 2021
Closed
@pllim
pllim requested review from bsipocz and embray July 8, 2021 18:46
@pllim pllim added the testing label Jul 8, 2021
@pllim

pllim commented Jul 8, 2021

Copy link
Copy Markdown
Member Author

Hmm. I think this warning is still a problem for "Python 3.7 with oldest supported version of all dependencies" job at least.

@bsipocz

bsipocz commented Jul 8, 2021

Copy link
Copy Markdown
Member

Hmm, it seems that with ignoring that error we masked over something similar in scipy as well?

@bsipocz

bsipocz commented Jul 8, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

I'm not sure what's going on, there is a similar old issue for scipy (scipy/scipy#11000), which concluded that it got resolved at some point between 1.0 and 1.3. However, I need 1.2 to get rid of the error, as opposed to an older version.

$ ~/munka/devel/astropy/.github [hacking ↓·3|…1⚑ 19] $ python -c 'import warnings; warnings.simplefilter("error", Warning); from scipy.interpolate import interpn'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/bsipocz/.pyenv/versions/3.7.9/lib/python3.7/site-packages/scipy/interpolate/__init__.py", line 175, in <module>
    from .interpolate import *
  File "/Users/bsipocz/.pyenv/versions/3.7.9/lib/python3.7/site-packages/scipy/interpolate/interpolate.py", line 32, in <module>
    from .interpnd import _ndim_coords_from_arrays
  File "interpnd.pyx", line 1, in init scipy.interpolate.interpnd
  File "/Users/bsipocz/.pyenv/versions/3.7.9/lib/python3.7/site-packages/scipy/spatial/__init__.py", line 96, in <module>
    from .qhull import *
  File "messagestream.pxd", line 5, in init scipy.spatial.qhull
  File "messagestream.pyx", line 5, in init scipy._lib.messagestream
ImportWarning: can't resolve package from __spec__ or __package__, falling back on __name__ and __path__
$ ~/munka/devel/astropy/.github [hacking ↓·3|…1⚑ 19] $ pip install numpy==1.17.* scipy==1.2
Requirement already satisfied: numpy==1.17.* in /Users/bsipocz/.pyenv/versions/3.7.9/lib/python3.7/site-packages (1.17.5)
Collecting scipy==1.2
  Using cached scipy-1.2.0-cp37-cp37m-macosx_10_6_intel.macosx_10_9_intel.macosx_10_9_x86_64.macosx_10_10_intel.macosx_10_10_x86_64.whl (28.7 MB)
Installing collected packages: scipy
  Attempting uninstall: scipy
    Found existing installation: scipy 1.1.0
    Uninstalling scipy-1.1.0:
      Successfully uninstalled scipy-1.1.0
Successfully installed scipy-1.2.0
WARNING: You are using pip version 20.1.1; however, version 21.1.3 is available.
You should consider upgrading via the '/Users/bsipocz/.pyenv/versions/3.7.9/bin/python3.7 -m pip install --upgrade pip' command.
$ ~/munka/devel/astropy/.github [hacking ↓·3|…1⚑ 19] $ python -c 'import warnings; warnings.simplefilter("error", Warning); from scipy.interpolate import interpn'
$ ~/munka/devel/astropy/.github [hacking ↓·3|…1⚑ 19] $

I suggest to update the minimum scipy 1.2, which is also more in line with the timelines for numpy, etc in APE18.

@pllim

pllim commented Jul 8, 2021

Copy link
Copy Markdown
Member Author

Thanks, @bsipocz ! That makes sense as the latest scipy is now 1.7. I pushed a commit to bump minversion as you suggested.

bsipocz
bsipocz approved these changes Jul 9, 2021
View reviewed changes

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is in line with the philosophy of APE18, and we already have a minimum numpy that is never than this scipy.

@pllim
pllim requested a review from saimn July 9, 2021 14:19
saimn
saimn approved these changes Jul 9, 2021
View reviewed changes

@saimn saimn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great to finally remove this warning filter !

@saimn
saimn merged commit 6e77e2c into astropy:main Jul 9, 2021
@pllim
pllim deleted the tst-unignore-cannot-resolve-package branch July 10, 2021 00:37
@bsipocz

bsipocz commented Aug 27, 2021

Copy link
Copy Markdown
Member

#11934 has bumped one more version of scipy, thus the changelog here is not obsolete and therefore deleted in that PR.

@pllim pllim mentioned this pull request Aug 27, 2021
Merged
@pllim

pllim commented Aug 27, 2021

Copy link
Copy Markdown
Member Author

Change log removed in #12120.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saimn saimn saimn approved these changes
@bsipocz bsipocz bsipocz approved these changes
@embray embray Awaiting requested review from embray

Assignees

No one assigned

Labels

Extra CI Run cron CI as part of PR installation no-changelog-entry-needed testing

Projects

None yet

Milestone

v5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@pllim @bsipocz @saimn