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

Stop using deprecated scipy namespaces #112

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

nicolascedilnik
Copy link
Contributor

A certain number of scipy.ndimage namespaces are deprecated, this PR ensures future-proofness.

@loli loli self-requested a review November 17, 2022 08:53
@loli loli self-assigned this Nov 17, 2022
Copy link
Owner

@loli loli left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

A short check revealed that there are few places where the syntax has not been changed yet:

./bin/medpy_fit_into_shape.py:from scipy.ndimage.interpolation import zoom
./bin/medpy_resample.py:    img = scipy.ndimage.interpolation.zoom(img, zoom_factors, order=args.order)
./bin/medpy_binary_resampling.py:from scipy.ndimage.interpolation import zoom
./bin/medpy_resample.py:import scipy.ndimage.interpolation
./medpy/filter/image.py:from scipy.ndimage.interpolation import zoom

Since they are all about the same method, I assume you left them out on purpose. Could you highlight why?

Then there are two places where private functions are used:

./medpy/features/intensity.py:from scipy.ndimage._ni_support import _get_output
./medpy/filter/image.py:from scipy.ndimage._ni_support import _get_output

I assume that these still require the old syntax. Correct me if I am wrong.

Once the first point has been clarified, I'll merge.

@nicolascedilnik
Copy link
Contributor Author

nicolascedilnik commented Nov 17, 2022

Thanks for the prompt review. The reason I had not changed the others is because I missed them (they are in scripts I don't use), but I agree, it's worth changing too, hence my force push.

About the private functions, I would not touch them, but I guess it would be better not to import anything private. Now I guess you had good reasons for importing them, but I would say that is a different issue?

@loli loli merged commit 66265de into loli:master Nov 17, 2022
@loli
Copy link
Owner

loli commented Nov 17, 2022

You are quite right about the private function. That's a matter for another issue. Thank you again!

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

2 participants