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

New function using matplotlib? #3624

Open
emmanuelle opened this issue Dec 24, 2018 · 7 comments
Open

New function using matplotlib? #3624

emmanuelle opened this issue Dec 24, 2018 · 7 comments

Comments

@emmanuelle
Copy link
Member

@ThomasWalter wrote a function plot_img for a new gallery example https://github.com/scikit-image/scikit-image/pull/2680/files#diff-76dba9af33e77a87351f24e67e6963fbR56, which displays a (small) grayscale image with user-provided values written on top of the pixels. This function could be useful for other examples, therefore it could be interesting to have it somewhere in the package. The only thing is that it uses matplotlib. Is it bad practice to have code importing matplotlib (it could be inside the function) now that matplotlib is an optional dependency? Could it go to the future submodule?

@hmaarrfk
Copy link
Member

I don't think it is bad to include functionality that depends on matplotlib, but I think one should be able to import scikit-image without having matplotlib installed.

The tricky aspect is developing the infrastructure to test it....

@tacaswell
Copy link
Contributor

Have a look at https://github.com/matplotlib/pytest-mpl for testing.

Long term I am interested in seeing a proliferation of domain-specific Matplotlib extensions, but that is still very vapor-warey.

@soupault
Copy link
Member

soupault commented Feb 2, 2019

From my perspective, the mentioned function is too trivial and doesn't worth to be included in the codebase. At least, until we have several gallery examples where it will be a proper visualisation mechanics.
If the team decides to include it, I'd vote to put it under future first, then moving to util.

@jni
Copy link
Member

jni commented Feb 3, 2019

I'm 👍 on including the function, I think it will be useful for many discussions in the docs about indexing, of which we need more. We can bikeshed about the API all we want. I also don't consider it trivial, having written my own a couple of times. It's kind of fiddly to get the scale, font, and centering all correct.

@ThomasWalter
Copy link
Contributor

I agree that this is always some fiddling, but here is the problem of the function also: the font and scale etc, are not adapted to the image size, so probably if used with a larger image the output would not be visually appealing. So if we include it, we should probably write it properly.

@stefanv
Copy link
Member

stefanv commented Feb 4, 2019

I'm not convinced this function is generic enough to be included as-is, but if we think through various use cases perhaps it could be. Should probably live in skimage.draw?

@jni
Copy link
Member

jni commented Feb 4, 2019

@stefanv it could be expanded quite a bit on a case-by-case basis. I wrote something similar for this blog post, and again for the skan paper:
https://ilovesymposia.com/2016/12/20/numba-in-the-real-world/

I agree that it needs a lot of generalising but it's doable in principle, that's what my 👍 for this issue was for.

Should probably live in skimage.draw?

Yes, I agree that this is the right place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants