Remove matplotlib as a core dependency. #3129

hmaarrfk · 2018-05-31T02:49:38Z

Issue #2710

Move matplotlib to optional dependency.
Skip tests that can't run without matplotlib

Few questions:

geometry.polygon_clip requires matplotlib path, transforms. :
- Look into updating the celiagg conda recipe so it isn't pinned to numpy 1.12. Ultimately, we decided against this since it seemed like we were just removing matplotliib and replacing it with a different dependency.
- See this PR Update to version 1.0.3 conda-forge/celiagg-feedstock#4 on celiagg's feedstock
- A python implemented of the required algorithm was written to replace the matplotlib exposed algorithm.

Note: This was rebased onto: #3432

Checklist

[It's fine to submit PRs which are a work in progress! But before they are merged, all PRs should provide:]

Clean style in the spirit of PEP8
Docstrings for all functions
Gallery example in ./doc/examples (new features only)
Unit tests

[For detailed information on these and other aspects see scikit-image contribution guidelines]

References

[If this is a bug-fix or enhancement, it closes issue # ]
[If this is a new feature, it implements the following paper: ]

For reviewers

(Don't remove the checklist below.)

Check that the PR title is short, concise, and will make sense 1 year
later.
Check that new functions are imported in corresponding __init__.py.
Check that new features, API changes, and deprecations are mentioned in
doc/release/release_dev.rst.

pep8speaks · 2018-05-31T02:49:45Z

Hello @hmaarrfk! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on February 28, 2019 at 04:14 Hours UTC

Undo some of the merge errors that were introduced in a previous rebase.

hmaarrfk · 2018-06-28T15:25:52Z

Future me notes: Seems like mac os builds might be failing because of a bug in libpng
ImageMagick/ImageMagick#747 (comment)

Borda · 2019-04-26T23:18:59Z

requirements/docs.txt

@@ -6,6 +6,7 @@ sphinx-gallery
 sphinx-copybutton
 pytest-runner
 scikit-learn
+matplotlib>=2.0.0,!=3.0.0


what about future >3.0.0 it still won't be compatible, right?

it should work, it is just 3.0.0 that is skipped. When 3.0.0 it had a bug, that was fixed in 3.0.1

well, now I see that there is no support for py2, (matplotlib 3.x droped it also)

for now, we still support installing matplotlib 2.3 + scikit-image 0.15

alexdesiqueira · 2019-05-24T23:14:36Z

Hey @hmaarrfk,
how are we on this one? :)

hmaarrfk · 2019-05-25T02:16:08Z

This needs a few core devs to want to take this on at one time. I can update if you want to review.

stefanv · 2019-12-06T00:30:50Z

skimage/_shared/_geometry.py

@@ -53,3 +61,81 @@ def polygon_area(pr, pc):
    pr = np.asarray(pr)
    pc = np.asarray(pc)
    return 0.5 * np.abs(np.sum((pc[:-1] * pr[1:]) - (pc[1:] * pr[:-1])))
+
+
+def _clip_sutherland_hodgman(rp, cp, rclip, cclip):


Where does this implementation come from? These clipping algorithms are notoriously tricky to get right.

I wrote it adapting the code you referred to I believe.

https://github.com/stefanv/supreme/blob/master/supreme/ext/polygon.c#L121

rfezzani · 2021-10-19T08:07:53Z

Sad that this PR is not merged 😕. @hmaarrfk do you have bandwidth to work again on it?

hmaarrfk · 2021-10-19T08:20:01Z

I'm not too sure it made sense to remove the core dependency without a concerted desire from the core devs.

To do so, requires effort to move cleanup PRs like this one forward.

And follow them up with other maintenance PRs.

I tink people can take the implementation of the function I wrote here verbatim. I think it works.

I think there remained one function that needed to be rewritten to truely get rid of matplotlib.

It remains to answer, why?

The scientific Python stack has gotten ALOT easier to install now. So I think that depending on matplotlib is only about saving disk space, and likely loading time in niche applications.

I think both of those can be accomplished by lazy loading, which I think there was an interesting PR by stefanv.

I do think it is good to reduce dependencies to make things faster to download. I've gotten bitten by 256k modems (or throttled to those speeds) and any python installation process was rather miserable. But I'm not sure it is a common enough usecase for me to devote time on.

rfezzani · 2021-10-19T08:37:14Z

Reducing our dependency may also help the adoption of skimage in safety-critical system development (medical software) where dependency and SOUP analysis is crucial!

hmaarrfk · 2021-10-19T09:09:48Z

That is very true. If you are willing to help push this through, then I can likely mustard some courage to revive certain PRs.

stefanv · 2021-10-20T06:06:56Z

I don't think matplotlib is software of unknown pedigree. It is true that we mostly use it for visualization, but it is employed in a few places for splines, polygons, etc. I'd get rid of the Qt dependency and deprecate skimage.viewer, then lazy load matplotlib as necessary (i.e., if you don't use it, it never gets pulled in).

rfezzani · 2021-10-20T07:33:53Z

@stefanv, even if matplotlib pedigree is well known, it's a huge dependency, and its risk analysis is far from being easy 😅.

then lazy load matplotlib as necessary (i.e., if you don't use it, it never gets pulled in).

Do you mean that it will be installed on user machine the first time a function involving it is called?

This PR supersedes #3129. It: - defines `skimage._shared.has_mpl` flag - uses `skimage._shared.version_requirements.require` decorator on the function using `matplotlib` - Disable `skimage.viewer` default imports if `matplotlib` is not installed - skip tests requiring `matplotlib` when it is not installed. Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org> Co-authored-by: Riadh <r.fezzani@vitadx.com>

jni · 2021-11-07T23:47:56Z

Superseded by #5990. Thank you for getting the ball rolling 3+ years ago (!), @hmaarrfk!

hmaarrfk changed the title ~~Lazy import a few matplotlib dependencies.~~ Remove matplotlib as a core dependency. May 31, 2018

soupault added the 🔧 type: Maintenance Refactoring and maintenance of internals label May 31, 2018

soupault added this to the 0.15 milestone May 31, 2018

This comment has been minimized.

Sign in to view

hmaarrfk force-pushed the feature_remove_matplotlib_dependency branch from 1b4c9e0 to cec0783 Compare May 31, 2018 22:55

hmaarrfk added a commit to hmaarrfk/scikit-image that referenced this pull request Jun 3, 2018

Invert some of the build logic in anticipation of PR scikit-image#3129

766b8ce

Undo some of the merge errors that were introduced in a previous rebase.

hmaarrfk force-pushed the feature_remove_matplotlib_dependency branch from 95ca24c to 4043434 Compare June 6, 2018 17:51

hmaarrfk added a commit to hmaarrfk/scikit-image that referenced this pull request Jun 7, 2018

Invert some of the build logic in anticipation of PR scikit-image#3129

0e725f4

Undo some of the merge errors that were introduced in a previous rebase.

hmaarrfk mentioned this pull request Jun 9, 2018

Modernise build process #3172

Closed

15 tasks

hmaarrfk force-pushed the feature_remove_matplotlib_dependency branch 3 times, most recently from c08a930 to 5ff38ab Compare June 28, 2018 05:09

hmaarrfk changed the title ~~Remove matplotlib as a core dependency.~~ WIP: Remove matplotlib as a core dependency. Jun 28, 2018

hmaarrfk mentioned this pull request Jun 28, 2018

Do not merge! Travis: explicitely run optional tests and QT test for OSX #3231

Closed

4 tasks

soupault self-assigned this Jul 29, 2018

hmaarrfk force-pushed the feature_remove_matplotlib_dependency branch from 14e7bf4 to 89d892a Compare August 6, 2018 03:14

hmaarrfk force-pushed the feature_remove_matplotlib_dependency branch 8 times, most recently from ec0795a to 3e2247c Compare September 30, 2018 05:09

hmaarrfk mentioned this pull request Sep 30, 2018

MAINT: Use upstream colormaps now that matplotlib has been upgraded #3429

Merged

9 tasks

hmaarrfk force-pushed the feature_remove_matplotlib_dependency branch 3 times, most recently from 41a6798 to ab6913a Compare September 30, 2018 14:03

hmaarrfk added 2 commits February 27, 2019 23:14

Build tools: Add a build without optional deps for linux python 3.6

d3549f4

CLeanup travis build matrix

b40aef6

hmaarrfk force-pushed the feature_remove_matplotlib_dependency branch from 5270a78 to b40aef6 Compare February 28, 2019 04:14

soupault modified the milestones: 0.15, 0.16 Apr 20, 2019

Borda reviewed Apr 26, 2019

View reviewed changes

alexdesiqueira added ⚖️ Needs decision and removed action: mrg+1 labels May 28, 2019

jni mentioned this pull request Jun 30, 2019

Add text rendering to skimage.draw #877

Closed

sciunto modified the milestones: 0.16, 1.0, 0.17 Jul 17, 2019

stefanv reviewed Dec 6, 2019

View reviewed changes

Base automatically changed from master to main February 18, 2021 18:23

mkcor mentioned this pull request Feb 22, 2021

2021's calendar of community management #5169

Closed

sofroniewn mentioned this pull request Mar 4, 2021

Use fancy indexing rather than full slices for label editing undo napari/napari#2339

Merged

10 tasks

grlee77 added the 🤖 type: Infrastructure CI, packaging, tools and automation label Apr 5, 2021

rfezzani mentioned this pull request Oct 26, 2021

Make matplotlib an optional dependency #5990

Merged

jni closed this Nov 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove matplotlib as a core dependency. #3129

Remove matplotlib as a core dependency. #3129

hmaarrfk commented May 31, 2018 •

edited

pep8speaks commented May 31, 2018 •

edited

This comment has been minimized.

hmaarrfk commented Jun 28, 2018

Borda Apr 26, 2019

hmaarrfk Apr 26, 2019

Borda Apr 27, 2019

hmaarrfk Apr 27, 2019

alexdesiqueira commented May 24, 2019

hmaarrfk commented May 25, 2019

stefanv Dec 6, 2019

hmaarrfk Dec 6, 2019

rfezzani commented Oct 19, 2021

hmaarrfk commented Oct 19, 2021

rfezzani commented Oct 19, 2021

hmaarrfk commented Oct 19, 2021

stefanv commented Oct 20, 2021

rfezzani commented Oct 20, 2021

jni commented Nov 7, 2021

Remove matplotlib as a core dependency. #3129

Remove matplotlib as a core dependency. #3129

Conversation

hmaarrfk commented May 31, 2018 • edited

Checklist

References

For reviewers

pep8speaks commented May 31, 2018 • edited

Comment last updated on February 28, 2019 at 04:14 Hours UTC

This comment has been minimized.

hmaarrfk commented Jun 28, 2018

Borda Apr 26, 2019

Choose a reason for hiding this comment

hmaarrfk Apr 26, 2019

Choose a reason for hiding this comment

Borda Apr 27, 2019

Choose a reason for hiding this comment

hmaarrfk Apr 27, 2019

Choose a reason for hiding this comment

alexdesiqueira commented May 24, 2019

hmaarrfk commented May 25, 2019

stefanv Dec 6, 2019

Choose a reason for hiding this comment

hmaarrfk Dec 6, 2019

Choose a reason for hiding this comment

rfezzani commented Oct 19, 2021

hmaarrfk commented Oct 19, 2021

rfezzani commented Oct 19, 2021

hmaarrfk commented Oct 19, 2021

stefanv commented Oct 20, 2021

rfezzani commented Oct 20, 2021

jni commented Nov 7, 2021

hmaarrfk commented May 31, 2018 •

edited

pep8speaks commented May 31, 2018 •

edited