Skip to content

Removing calls to skimage.viewer and switching to "lazy importing" module import #99

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 10 commits into from
Aug 12, 2021

Conversation

erickmartins
Copy link
Contributor

The Image Viewer submodule for scikit-image is being deprecated in 0.18 and removed in 0.20 (https://scikit-image.org/docs/dev/user_guide/viewer.html) - I've gone through the material (both episodes and example scripts) and changed all calls to skimage.viewer.ImageViewer with calls to skimage.io.imshow. Furthermore, any instances of import skimage have been replaced by specific imports for specific modules.

The Edge Detection episode has a lengthy section that uses the skimage.viewer Plugins feature - replacing that whole section felt outside the scope of this PR, so I've left it as is for the moment. It is probably a good idea to put a rewrite of that section using something else (napari?) in the roadmap.

Copy link
Member

@tobyhodges tobyhodges left a comment

Choose a reason for hiding this comment

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

Thanks for submitting these changes @erickmartins. I agree that this is probably the best approach to take, though there are some further changes that will need to be made to the lesson afterwards, e.g. adding a callout box about interactive image viewers and including the output of calls to skimage.io.imshow in the lesson pages.

I only caught one change that still needs making, where the code block should be updated to match the adjusted explanatory text. See the suggestion included in my review. Once that is committed I will be ready to merge this.

Co-authored-by: Toby Hodges <tbyhdgs@gmail.com>
@erickmartins
Copy link
Contributor Author

Should be done! Thanks.

@tobyhodges tobyhodges self-requested a review April 13, 2021 13:32
Copy link
Member

@tobyhodges tobyhodges left a comment

Choose a reason for hiding this comment

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

Thanks @erickmartins. I will leave the PR unmerged for the next 24 hours, in case any of the other authors want to discuss this (fairly large) change further.

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.

3 participants