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

RF: Centralize import of decorators from numpy.testing #738

Merged
merged 3 commits into from Mar 13, 2019

Conversation

yarikoptic
Copy link
Member

I kept receiving messages that imports from numpy.testing.decorators are
deprecated (since 1.5 IIRC), so I found that some places still import them
in the test files instead of centraly from nibabel.testing which already
provides imports adaptor. So I RFed to avoid such imports, and moved a
stub for @slow there too

I kept receiving messages that imports from numpy.testing.decorators are
deprecated (since 1.5 IIRC), so I found that some places still import them
in the test files instead of centraly from nibabel.testing which already
provides imports adaptor.  So I RFed to avoid such imports, and moved a
stub for @slow there too
@coveralls
Copy link

coveralls commented Mar 8, 2019

Coverage Status

Coverage increased (+0.0007%) to 91.917% when pulling 0c36551 on yarikoptic:enh-deprec-numpy.testing.decorators into ad0b13f on nipy:master.

Copy link
Member

@effigies effigies 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 reasonable. I'll try to look at the test failures...

except ImportError:
from numpy.testing.decorators import skipif

# Recent (1.2) versions of numpy have this decorator
Copy link
Member

Choose a reason for hiding this comment

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

We have a minimum version of 1.7.1. I don't think this is guard is necessary anymore.

@effigies
Copy link
Member

effigies commented Mar 8, 2019

Oh, the test failure is just the usual AppVeyor nonsense. I thought there was a problem with a recent version of matplotlib. Might be thinking of another project.

@codecov-io
Copy link

codecov-io commented Mar 8, 2019

Codecov Report

Merging #738 into master will decrease coverage by 3.14%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #738      +/-   ##
==========================================
- Coverage   89.39%   86.24%   -3.15%     
==========================================
  Files          93       93              
  Lines       11476    11474       -2     
  Branches     1992     1992              
==========================================
- Hits        10259     9896     -363     
- Misses        881     1252     +371     
+ Partials      336      326      -10
Impacted Files Coverage Δ
nibabel/testing/__init__.py 96.8% <100%> (+2.01%) ⬆️
nibabel/dft.py 15.85% <0%> (-65.25%) ⬇️
nibabel/freesurfer/io.py 51.54% <0%> (-42.74%) ⬇️
nibabel/pydicom_compat.py 65% <0%> (-35%) ⬇️
nibabel/environment.py 75% <0%> (-20%) ⬇️
nibabel/pkg_info.py 65.51% <0%> (-13.8%) ⬇️
nibabel/casting.py 81.35% <0%> (-5.94%) ⬇️
nibabel/openers.py 78.22% <0%> (-3.23%) ⬇️
nibabel/__init__.py 91.48% <0%> (-2.13%) ⬇️
nibabel/volumeutils.py 91.78% <0%> (-0.98%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad0b13f...c053e41. Read the comment docs.

@effigies effigies added this to the 2.4.0 milestone Mar 8, 2019
@effigies effigies changed the title RF: centralize import of decorators from numpy.testing RF: entralize import of decorators from numpy.testing Mar 8, 2019
@effigies effigies changed the title RF: entralize import of decorators from numpy.testing RF: Centralize import of decorators from numpy.testing Mar 8, 2019
except ImportError:
from numpy.testing.decorators import skipif
from numpy.testing.decorators import (skipif, slow)
Copy link
Member

Choose a reason for hiding this comment

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

Something's weird with the testing because this is never getting hit, even on the numpy==1.7.1 build. See 2007.5, here:

https://codecov.io/gh/nipy/nibabel/commit/0c36551c7398dcfb132fe4ce37b115c23d183b72/build

I know it's not really relevant to this PR, but any thoughts? I checked the travis log, and see no indication that anything other than 1.7.1 was installed.

Copy link
Member Author

Choose a reason for hiding this comment

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

No immediate idea, I am away from the laptop ATM, but if I were with it I would remove try/except and see how it fails

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it's because numpy.testing.dec has existed forever. It's just an alias of an imported module, and the module it's aliasing has changed from numpy.testing.decorators to numpy.testing._private.decorators.

How about we just drop the try/except bit, and load dec, skipif, and slow?

@effigies effigies merged commit 4f7d3d5 into nipy:master Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants