search

scikit-image / scikit-image

Image processing in Python
https://scikit-image.org
Other
6.08k stars 2.23k forks source link

New function using matplotlib? #3624

Open emmanuelle opened 5 years ago

emmanuelle commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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.