Skip to content

[REF] Deprecate kw_only_meth/func decorators #848

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

Merged
merged 3 commits into from
Dec 19, 2019

Conversation

rmarkello
Copy link
Contributor

Closes #841.

This is a minor update, but it removes a bit of custom code (the nibabel.keywordonly module) forcing methods/functions to have a pre-specified number of positional arguments and instead uses the Python 3.x * argument to achieve the same performance.

I haven't added any additional tests to check that the * argument performs identically to the decorators since test_keywordonly.py was only testing the decorators themselves (and not the functions they wrapped)—and those functions are now removed—but let me know if you'd like to see something explicit!

Uses built-in Python kw-only syntax (*) instead.
@codecov
Copy link

codecov bot commented Dec 6, 2019

Codecov Report

Merging #848 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #848      +/-   ##
==========================================
- Coverage   90.09%   90.07%   -0.02%     
==========================================
  Files          98       98              
  Lines       12452    12428      -24     
  Branches     2190     2190              
==========================================
- Hits        11219    11195      -24     
  Misses        883      883              
  Partials      350      350
Impacted Files Coverage Δ
nibabel/freesurfer/mghformat.py 95.52% <100%> (-0.04%) ⬇️
nibabel/keywordonly.py 100% <100%> (ø) ⬆️
nibabel/dataobj_images.py 95.58% <100%> (-0.19%) ⬇️
nibabel/ecat.py 88.2% <100%> (-0.07%) ⬇️
nibabel/cifti2/cifti2.py 96.67% <100%> (-0.02%) ⬇️
nibabel/arrayproxy.py 100% <100%> (ø) ⬆️
nibabel/parrec.py 91.78% <100%> (-0.08%) ⬇️
nibabel/analyze.py 98.52% <100%> (-0.01%) ⬇️
nibabel/minc2.py 89.87% <100%> (-0.26%) ⬇️
nibabel/minc1.py 90.9% <100%> (-0.11%) ⬇️
... and 2 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 fcc5448...e69ec32. Read the comment docs.

@rmarkello
Copy link
Contributor Author

Also, in case it needs to be said: this is very low priority and can just hang out until after the 3.0 release! I just had a few minutes and though I'd get it off my to-do list 😅

@effigies
Copy link
Member

effigies commented Dec 6, 2019

Since this is part of the API, I'm afraid it will need to go through a deprecation cycle.

Have a look at:

nibabel/nibabel/minc.py

Lines 1 to 10 in fcc5448

""" Deprecated MINC1 module """
import warnings
warnings.warn("We will remove this module from nibabel 3.0; "
"Please use the 'minc1' module instead",
DeprecationWarning,
stacklevel=2)
from .minc1 import * # noqa

MODULE_SCHEDULE = [
('4.0.0', ['nibabel.trackvis']),
('3.0.0', ['nibabel.minc', 'nibabel.checkwarns']),
# Verify that the test will be quiet if the schedule outlives the modules
('1.0.0', ['nibabel.neverexisted']),
]

@rmarkello
Copy link
Contributor Author

Gotcha! Do you have a preference for what version to list for deprecation? I'm assuming 4.0.0 based on what's currently listed.

@effigies
Copy link
Member

effigies commented Dec 6, 2019

I think we usually go +2 major versions, so let's say 5.0.

Marked for removal in version 5.0.0
@effigies effigies added this to the 3.1.0 milestone Dec 6, 2019
@effigies effigies changed the title [REF] Removes kw_only_meth/func decorators [REF] Deprecate kw_only_meth/func decorators Dec 6, 2019
@effigies
Copy link
Member

Thanks!

@effigies effigies merged commit 6425c2a into nipy:master Dec 19, 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.

Replace @kw_only_meth/func decorator with "*" argument
2 participants