Change old import convention in the gallery examples by jgyasu · Pull Request #7630 · scikit-image/scikit-image

jgyasu · 2024-12-11T08:37:11Z

Old import convention is used in the gallery examples, this PR changes it to new import convention in all the examples on the applications directory

Resolves part of #7454.

Description

Checklist

A descriptive but concise pull request title
Docstrings for all functions
Unit tests
A gallery example in ./doc/examples for new features
Contribution guide is followed

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

...

Old import convention is used in the gallery examples, this PR changes it to new import convention in all the examples on the applications directory Resolves: scikit-image#7454

jgyasu · 2024-12-11T10:23:12Z

Hi, @mkcor, based on the comments in #7454, I made the changes in all the examples of docs/examples/applications, I ran the pre-commit on my local machine and passed all the checks but now I get this pre-commit.ci failure. Can you please help?

lagru · 2024-12-11T12:36:55Z

but now I get this pre-commit.ci failure

That automatic check makes sure that the code is formatted properly and also checks for common errors. If you click on the job's "Details" you can see the result.

The check fails because the file doc/source/release_notes/release_dev.rst isn't formatted properly. That's a file you haven't touched, and that error was caused by us. I'll fix it in a separate PR. For now, you don't need to worry about it. 😉

Schefflera-Arboricola · 2024-12-11T12:44:27Z

fixed in #7631

jgyasu · 2024-12-11T12:44:41Z

but now I get this pre-commit.ci failure

That automatic check makes sure that the code is formatted properly and also checks for common errors. If you click on the job's "Details" you can see the result.

The check fails because the file doc/source/release_notes/release_dev.rst isn't formatted properly. That's a file you haven't touched, and that error was caused by us. I'll fix it in a separate PR. For now, you don't need to worry about it. 😉

Ah, thank you! This makes sense now, I was confused. I chose this issue since I'm a first time contributor to Scikit-image and now that the development environment is set, I can contribute to more issues. :)

lagru

Thank you @jgyasu for working on this. It looks like you still missed a few files in the doc/examples/applications/ directory. E.g., plot_coins_segmentation.py.

You can easily find those cases by searching specifically for "skimage." (notice the dot) in the applications/ directory. For example with

grep "skimage." -R doc/examples/applications/

jgyasu · 2024-12-11T14:01:49Z

Thank you @jgyasu for working on this. It looks like you still missed a few files in the doc/examples/applications/ directory. E.g., plot_coins_segmentation.py.

You can easily find those cases by searching specifically for "skimage." (notice the dot) in the applications/ directory. For example with
grep "skimage." -R doc/examples/applications/

grep was really helpful, I earlier used VS Code's find and replace function and missed a few instances of the old-convention imports

mkcor

Thank you for getting started with scikit-image contributions, @jgyasu! Welcome aboard!

…kit-image into new-import-convention

mkcor · 2024-12-14T21:37:42Z

 # The following example shows how local auto-level enhances the camara man
 # picture.

-from skimage.filters.rank import autolevel


Here and in other places, you typically want to remove a blank line as well (either the one above or the one below) in addition to the import line. Otherwise you end up with an extra blank line.

Thank you, I will fix it. I contributed to an another open-source project once and it used pre-commit and before committing, it automatically edited out all the style issues. I want to know if there exists any tool like that for Scikit-image?

Hi @jgyasu,

Yes, "we recommend that you use a pre-commit hook, which runs code checkers and formatters each time you do a git commit" (Development process). It looks like you have installed it, and that it ran successfully, since CI says:

pre-commit.ci - pr — checks completed successfully

mkcor · 2024-12-14T21:48:47Z

@jgyasu don't worry about the CI failure, it's unrelated!

Looking at the failing workflow, I can see: matplotlib-3.10.0 and AttributeError: 'QuadContourSet' object has no attribute 'collections'. A quick search gave: “MatplotlibDeprecationWarning: The collections attribute was deprecated in Matplotlib 3.8 and will be removed two minor releases later” (from https://discourse.matplotlib.org/t/collections-attribute-deprecation-in-version-3-8/24164).

I'll open an issue.

jgyasu · 2024-12-15T04:34:21Z

Also @mkcor is there any issue or feature that I can work on? I want a good understanding of the codebase and I'm ready to work on something even if it takes time

mkcor · 2024-12-20T17:49:24Z

Hi @jgyasu,

I have updated your branch with the latest changes in main so CI should pass (fix was #7638).

Also @mkcor is there any issue or feature that I can work on? I want a good understanding of the codebase and I'm ready to work on something even if it takes time

Thank you for this generous offer! 🙏

Here we have a list of changes we would like to implement (independently of the skimage2 migration/transition). You are more than welcome to pick one, start/continue a discussion in an issue, and/or submit a (draft) PR. Feel free to ping us to get feedback.

For example, one goal is to make the default behavior of all our thresholding functions consistent. It's not something that will happen over a quick PR! Also, you'll have to learn how to deprecate a function or some of its parameters: See https://github.com/scikit-image/scikit-image/pull/7225/files for example.

I hope the learning curve isn't too steep. That would be a great investment for future contributions, for sure! Thanks again.

mkcor · 2025-01-04T17:46:03Z

The CI failure for linux-cp3.13-default is reminiscent of what #7612 should have fixed:

FileNotFoundError: [Errno 2] No such file or directory: '/home/runner/work/scikit-image/scikit-image/doc/examples/applications/plot_3d_interaction.png'

That thumbnail is generated from a Plotly figure. Although, from https://pypi.org/project/plotly/,

I'm surprised to read that "The kaleido package has no dependencies..." 🤔

lagru

I left a minor nitpick, but looks ready to merge otherwise. Thanks @jgyasu. :)

I'll trigger another CI check just to make sure that the failures are unrelated.

mkcor · 2025-01-06T17:31:04Z

I guess there's a change with Python 3.12, perhaps when specifying a random seed...? The failing test is

scikit-image/skimage/graph/tests/test_rag.py

Lines 209 to 211 in efe339b

    
           def test_reproducibility(): 
        
               """ensure cut_normalized returns the same output for the same input, 
        
               when specifying random seed

and the error only occurs with Python 3.12: "FAILED graph/tests/test_rag.py::test_reproducibility - AssertionError."

Locally, for me,

spin test skimage/graph/tests/test_rag.py::test_reproducibility

passes either on main or this feature branch (I fetched it), but I'm running Python 3.11 precisely.

Edit: Nevermind, current CI is green on main and it also runs Python 3.12 (even 3.13), so that can't be it. 😓

lagru · 2025-01-08T14:23:01Z

@mkcor I made an issue for what you describe in your comment directly above. It's #7651.

mkcor · 2025-01-08T15:10:05Z

Thanks, @lagru. Ok, I'll approve and merge, since the CI failures are unrelated (and still mysterious).

mkcor · 2025-01-08T15:17:00Z

@jgyasu congrats on getting your first scikit-image contribution merged! 🎉

jgyasu · 2025-01-09T04:44:03Z

@jgyasu congrats on getting your first scikit-image contribution merged! 🎉

Thank you! After my exams, I hope to contribute more 🙂

stefanv · 2025-01-09T07:42:59Z

🙏 👏

jgyasu added 2 commits December 11, 2024 14:03

Change old import convention in the gallery examples

51c5787

Old import convention is used in the gallery examples, this PR changes it to new import convention in all the examples on the applications directory Resolves: scikit-image#7454

Change ski.cells3d() to ski.data.cells3d()

80b2c39

mkcor added the 📄 type: Documentation Updates, fixes and additions to documentation label Dec 11, 2024

lagru reviewed Dec 11, 2024

View reviewed changes

Change old import convention in instances left in the first commit

bb86b0e

Merge branch 'main' into new-import-convention

276187c

jgyasu requested a review from lagru December 11, 2024 15:16

mkcor reviewed Dec 14, 2024

View reviewed changes

jgyasu added 3 commits December 14, 2024 23:53

Merge remote-tracking branch 'upstream/main' into new-import-convention

65bff43

Remove repeated skimage imports in the examples

b8862a4

Merge branch 'new-import-convention' of https://github.com/jgyasu/sci…

fabdd66

…kit-image into new-import-convention

mkcor reviewed Dec 14, 2024

View reviewed changes

mkcor mentioned this pull request Dec 14, 2024

Gallery example on morphological snakes uses removed attribute 'collections.' #7636

Closed

Merge branch 'main' into new-import-convention

5aa9af6

lagru approved these changes Jan 5, 2025

View reviewed changes

Comment thread doc/examples/applications/plot_rank_filters.py Outdated

lagru and others added 2 commits January 5, 2025 15:16

Update doc/examples/applications/plot_rank_filters.py

4b8ecac

Prevent ruff from reflowing lines due to comma

deba9f5

mkcor approved these changes Jan 8, 2025

View reviewed changes

mkcor merged commit e514cc9 into scikit-image:main Jan 8, 2025

stefanv added this to the 0.25.1 milestone Jan 8, 2025

mkcor mentioned this pull request Jan 8, 2025

Use matplotlib oo-style in examples #7531

Merged

Uh oh!

Conversation

jgyasu commented Dec 11, 2024 • edited by lagru Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Release note

Uh oh!

jgyasu commented Dec 11, 2024

Uh oh!

lagru commented Dec 11, 2024

Uh oh!

Schefflera-Arboricola commented Dec 11, 2024

Uh oh!

jgyasu commented Dec 11, 2024

Uh oh!

lagru left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgyasu commented Dec 11, 2024

Uh oh!

mkcor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkcor Dec 14, 2024

Choose a reason for hiding this comment

Uh oh!

jgyasu Dec 15, 2024

Choose a reason for hiding this comment

Uh oh!

mkcor Jan 4, 2025

Choose a reason for hiding this comment

Uh oh!

mkcor commented Dec 14, 2024

Uh oh!

jgyasu commented Dec 15, 2024

Uh oh!

mkcor commented Dec 20, 2024

Uh oh!

mkcor commented Jan 4, 2025

Uh oh!

lagru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mkcor commented Jan 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lagru commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkcor commented Jan 8, 2025

Uh oh!

mkcor commented Jan 8, 2025

Uh oh!

jgyasu commented Jan 9, 2025

Uh oh!

stefanv commented Jan 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jgyasu commented Dec 11, 2024 •

edited by lagru

Loading

lagru left a comment •

edited

Loading

mkcor commented Jan 6, 2025 •

edited

Loading

lagru commented Jan 8, 2025 •

edited

Loading