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

Remove the Deprecating nisext #2800

Merged
merged 1 commit into from
May 3, 2023
Merged

Conversation

skoudoro
Copy link
Member

@skoudoro skoudoro commented May 3, 2023

Nibabel has deprecated nisext. We were almost not using those tools. After checking, only check-files was used sometimes. Also, those tools depends on distutils that we plan to remove completely.

This is why this PR remove completely nisext since this is a package that we will not use in a close future. it also allow Nibabel to move forward.

Thanks for all the good services nisext!

closes #2706

Copy link
Contributor

@arokem arokem left a comment

Choose a reason for hiding this comment

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

LGTM. One small and not crucial comment, but otherwise, I think you can merge this when it comes back green from the CI.

@@ -1,7 +1,5 @@
""" Distutils / setuptools helpers for versioning

This code started life in the nibabel package as nisexts/sexts.py
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest leaving this comment in, maybe adding a comment that it has been deprecated upstream in nibabel.

Copy link
Member Author

Choose a reason for hiding this comment

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

This file will be removed before the end of the summer (september). This is a way to start the process 😄

@codecov
Copy link

codecov bot commented May 3, 2023

Codecov Report

Merging #2800 (ed03618) into master (f035a0b) will decrease coverage by 0.02%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2800      +/-   ##
==========================================
- Coverage   81.49%   81.47%   -0.02%     
==========================================
  Files         143      143              
  Lines       20051    20051              
  Branches     3192     3192              
==========================================
- Hits        16340    16336       -4     
- Misses       2904     2907       +3     
- Partials      807      808       +1     

see 1 file with indirect coverage changes

@skoudoro
Copy link
Member Author

skoudoro commented May 3, 2023

Thank you @arokem for the review. All green, merging

@skoudoro skoudoro merged commit 3749aa2 into dipy:master May 3, 2023
27 checks passed
@skoudoro skoudoro deleted the remove-nisext branch May 3, 2023 18:53
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.

FYI: Deprecating nisext in nibabel
2 participants