Hello @mkcor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file skimage/data/_registry.py:

Line 133:80: E501 line too long (90 > 79 characters)
Line 134:80: E501 line too long (88 > 79 characters)
Line 141:80: E501 line too long (106 > 79 characters)
Line 142:80: E501 line too long (109 > 79 characters)

Comment last updated at 2020-09-02 19:26:18 UTC

@emmanuelle fyi, and tagging @jni for review

I vote for "lily" and "kidney" as names, it's a good idea to shorten them.

I vote for "lily" and "kidney" as names, it's a good idea to shorten them.

Sold! Done f70dbf9. Yes, this way they're also in line with the other datasets in scikit-image, which are named with one word: cells, eagle, mitosis, ... And if they ever become famous, the world will talk about "the kidney image" just like "the iris dataset," you know. 😆

Thanks for weighing in, @GenevieveBuckley!

A function must be added and we need to had an example in examples/data. For the gallery, a new page must be created for 3D dataset and we need to think how we can provide a preview -- we do not have the ability to interact with images yet.

we need to think how we can provide a preview -- we do not have the ability to interact with images yet.

@sciunto tomorrow I want to try something out with plotly :-)

we need to think how we can provide a preview -- we do not have the ability to interact with images yet.

@sciunto tomorrow I want to try something out with plotly :-)

Cool! :) (In case you haven't seen it: #4762)

we need to think how we can provide a preview -- we do not have the ability to interact with images yet.

@sciunto tomorrow I want to try something out with plotly :-)

You could also load the image into napari, switch it to 3D view and then take a screenshot (I had to split the color channels into separate layers to give them different colormaps). The kidney tissue is a pretty thin slice, so it doesn't look quite as impressive in 3D as other datasets might. Feel free to use these in the docs if they work for you.

I don't know if you already have images in the docs for the brain image too, but it also looks pretty cool visualized in napari

Axial plane

3D view

It's a good idea, but we want to do not commit images to the main repository anymore.
Either the preview you suggest is added to the data repository, or we work on a visualization method for our documentation, or we make a simple 2D view and we improve it when we are ready to make nice views in our doc.

my taste for small incremental steps goes for the last solution.

my 2 cents.

I agree with @sciunto, we want to create our views programmatically from the data within our docs. Axis-aligned max projection is easy enough to do for now, and we can add something like the plotly Volume render eventually: https://plotly.com/python/3d-volume-plots/ (Though I don't really love those visuals...)

@GenevieveBuckley and if you meant run napari within the docs, let's first figure that out in the napari docs! 😂 Also we don't yet have a camera model, though Nick has that as his next priority.

Cool! :) (In case you haven't seen it: #4762)

Indeed, thanks for the pointer!

we need to {have, add} an example in examples/data

@sciunto not sure I understand correctly: Why should the example(s) necessarily fall under https://scikit-image.org/docs/stable/auto_examples/#data? I was taking a stab at an 'application' example ("Longer examples and demonstrations"): #4946

A function must be added

@sciunto do you mean data.kidney() and data.lily()? I didn't realize that! I guess we would also want data.mitosis() and data.cells(), which is not available at the moment:

A function must be added

@sciunto do you mean data.kidney() and data.lily()? I didn't realize that! I guess we would also want data.mitosis() and data.cells(), which is not available at the moment:

scikit-image/skimage/data/tests/test_data.py

Lines 149 to 153 in 9e41809

	def test_cells_3d():
	"""Needs internet connection."""
	path = fetch('data/cells.tif')
	image = io.imread(path)
	assert image.shape == (60, 256, 256)

Absolutely. The aim of this page is to display the content of our dataset

@GenevieveBuckley and if you meant run napari within the docs, let's first figure that out in the napari docs! 😂 Also we don't yet have a camera model, though Nick has that as his next priority.

Nope, I didn't mean that! We're in agreement here.

I did not understand earlier that we wanted the figures/images for the docs to be auto-generated.

A function must be added and we need to had an example in examples/data. For the gallery, a new page must be created for 3D dataset and we need to think how we can provide a preview -- we do not have the ability to interact with images yet.

I agree with this statement.

The file you are looking for is here:
https://github.com/scikit-image/scikit-image/blob/master/skimage/data/__init__.py

https://github.com/scikit-image/scikit-image/blob/master/skimage/data/__init__.py

@hmaarrfk yes, yes, spotted it! Thanks -- taking care of it now.

https://github.com/scikit-image/scikit-image/blob/master/skimage/data/__init__.py

@hmaarrfk yes, yes, spotted it! Thanks -- taking care of it now.

Done. /cc @sciunto

@hmaarrfk a3c73c2 I went the verbose/copying-pasting route you suggested.

Thanks @mkcor and reviewers! Could you just add lily to https://github.com/scikit-image/scikit-image/blob/master/doc/examples/data/plot_scientific.py#L16 before we merge this PR?

Thanks @mkcor and reviewers! Could you just add lily to https://github.com/scikit-image/scikit-image/blob/master/doc/examples/data/plot_scientific.py#L16 before we merge this PR?

Sure! Done 973508e. Good to know of this list.

Merging, thanks !

We have some pretty bad CI failures :-(. But they are unrelated to this PR.