Blob Detection Example #986

vighneshbirodkar · 2014-04-22T19:52:05Z

@cdeil
The Hubble Deep Field image is added here.

My only concern is the example runs 3 algorithms, and is unresponsive for 6s before all 3 images pop up.

vighneshbirodkar · 2014-04-22T20:27:59Z

matplotlib can't load a jpg image because there is no there is no PIL on Python3. Should I resubmit this with a PNG image ?

cdeil · 2014-04-22T21:21:05Z

matplotlib can't load a jpg image because there is no there is no PIL on Python3.

Being able to read JPG (via some external library) is pretty fundamental for an image-processing library like scikit-image. What is the recommended method for Python 3 for scikit-image users?

cdeil · 2014-04-22T21:25:16Z

doc/examples/plot_blob.py

+
+blobs = blob_log(image_gray, max_sigma=30, num_sigma=10, threshold=.1)
+
+fig = plt.figure()


You basically repeat the same code three times.
Put it in a function and call that three times?

If you notice Line 85 is different than the other 2 cases.

Then compute the three types of blobs first, multiply the one by sqrt(2), then proceed with a for loop over all three.

vighneshbirodkar · 2014-04-22T21:31:27Z

I am not sure what the recommended way is on Python3 but the freeimage plugin can do it

cdeil · 2014-04-22T21:38:26Z

Let's wait for the scikit-image devs to comment if JPG is OK for an example image.
If yes you can simply update the .travis.yml file to make it work on Python 3.

With a very quick search I couldn't find info on how to read JPG images in the scikit-image docs.
If it's not there, @vighneshbirodkar, maybe you could add it?

stefanv · 2014-04-22T21:38:35Z

FreeImage is installed on Travis, and should automatically be picked up. But even Matplotlib should be able to read JPEG, so not sure why this won't work on 3.

stefanv · 2014-04-22T21:39:57Z

Ah, it's actually Matplotlib raising the error.

vighneshbirodkar · 2014-04-22T21:42:24Z

I guess Travis hates us for some reason 😟
I think we can fix this by giving FreeImage priority whenever it's installed

stefanv · 2014-04-22T21:42:53Z

Perhaps FreeImage should become the preferred default plugin:

https://github.com/scikit-image/scikit-image/blob/master/skimage/io/manage_plugins.py#L47

Alternatively, disable the matplotlib plugin when PIL is unavailable (you can see how to do it in the freeimage plugin).

stefanv · 2014-04-22T21:44:01Z

doc/examples/plot_blob.py

+
+blobs = blob_doh(image_gray, max_sigma=30, threshold=.01)
+
+fig = plt.figure()


We prefer the following syntax:

f, ax = plt.subplots(1, 1)

JDWarner · 2014-04-22T22:34:32Z

We should probably default away from matplotlib for Python 3, at least until they decide Pillow is stable enough to incorporate or PIL comes back from the dead with a Python 3 compatible version...

vighneshbirodkar · 2014-04-23T17:42:07Z

FreeImage cannot read a bytestream. Should I use a specific plugin in the failing test case ?

tacaswell · 2014-04-23T22:58:54Z

(mpl dev) We have already switched to defaulting to pillow (atleast on the master branch matplotlib/matplotlib#2137 )

stefanv · 2014-04-25T07:20:26Z

Why don't we just update our PIL plugin to also support pillow?

jni · 2014-04-25T12:58:13Z

@stefanv I thought there was a recent PR to do that? I think it was
@sciunto's, and I merged it.

On Fri, Apr 25, 2014 at 5:20 PM, Stefan van der Walt <
notifications@github.com> wrote:

Why don't we just update our PIL plugin to also support pillow?

Reply to this email directly or view it on GitHubhttps://github.com//pull/986#issuecomment-41365271
.

stefanv · 2014-04-25T13:51:22Z

Ah! If so, we probably only need to install pillow on the Python 3
buildbot.

vighneshbirodkar · 2014-04-26T12:07:51Z

Wouldn't it be better to install pillow for both Python 2 and Python 3 ?

stefanv · 2014-04-26T13:01:56Z

Sure, that's fine too.

coveralls · 2014-04-26T16:53:40Z

Coverage remained the same when pulling 75ed73d on vighneshbirodkar:blob_example into 7a1213a on scikit-image:master.

stefanv · 2014-04-26T17:06:49Z

skimage/data/__init__.py

+def hubble_deep_field():
+    """Hubble eXtreme Deep Field
+
+    This photograph contains the Hubble Telescopes farthest ever view of


Hubble Space Telescope's

coveralls · 2014-04-26T20:02:39Z

Coverage remained the same when pulling 5a67937 on vighneshbirodkar:blob_example into 7a1213a on scikit-image:master.

stefanv · 2014-04-27T01:58:58Z

Looks good! Please fix the typos in the commit messages. Thereafter, I'll let @cdeil sign off on the changes.

cdeil · 2014-04-27T14:25:45Z

@stefanv FYI: Probably @vighneshbirodkar doesn't have permissions to re-start the travis-ci build. There's a feature request to let committers do this, but it's not available at the moment: travis-ci/travis-ci#887.

cdeil · 2014-04-27T14:28:18Z

doc/examples/plot_blob.py

+Blobs are bright on dark or dark on bright regions in an image. In
+this example, blobs are detected using 3 algorithms. The image used
+in this case is the Hubble eXtreme Deep Field. Each bright dot in the
+image is a star or a galaxy, so we are literally counting stars.


This is not true (I think most blobs are Galaxies, not stars), so I suggest to remove this statement:

so we are literally counting stars.

cdeil · 2014-04-27T14:46:22Z

@vighneshbirodkar I've left some minor inline comments.

Is it possible to link to API docs from the example text?
If yes, I think the example would benefit from linking to the blob_* and hubble_deep_field docstrings.

stefanv · 2014-04-27T14:59:14Z

Thanks for the detailed feedback, @cdeil!

vighneshbirodkar · 2014-04-27T19:06:25Z

skimage/data/__init__.py

+    <http://hubblesite.org/newscenter/archive/releases/2012/37/image/a/>`__.
+
+    The image was captured by NASA and `may be freely used in the
+    public domain <http://www.nasa.gov/audience/formedia/features/MP_Photo_Guidelines.html>`_.


@cdeil The URL itself occupies 80 chars. IMHO we should not break the URL.

coveralls · 2014-04-27T19:22:22Z

Coverage remained the same when pulling 580f5d2 on vighneshbirodkar:blob_example into 7a1213a on scikit-image:master.

vighneshbirodkar · 2014-04-27T19:25:28Z

@cdeil , @stefanv
Thanks a lot for the feedback. And sorry for my typos.

cdeil · 2014-04-27T20:13:28Z

@vighneshbirodkar Thanks for taking all the little wording comments into account.

I think there's one comment I wrote above you haven't taken into account (or rejected) yet:

Is it possible to link to API docs from the example text?
If yes, I think the example would benefit from linking to the blob_* and hubble_deep_field docstrings.

Once that is addressed, 👍 to merge.

I would prefer this slightly different way of writing the example:
https://gist.github.com/cdeil/11354543
But that's a matter of taste, so @vighneshbirodkar, you decide.

vighneshbirodkar · 2014-04-27T20:30:26Z

@cdeil
It might not always be true that the radius is a a multiple of the 3rd parameter returned.

cdeil · 2014-04-27T20:34:40Z

It might not always be true that the radius is a a multiple of the 3rd parameter returned.

Why not?
I guess the methods you are thinking of measure an area, but for API uniformity I would suggest to return sqrt(area) in those cases and have the third parameter always be proportional to the radius.

vighneshbirodkar · 2014-04-27T20:41:11Z

We could compute and always return the radius of the blob, that was how I initially implemented it, but later on it was decided that returning the "model parameters" was the better way, as @stefanv suggested

Edit : See #903

coveralls · 2014-04-27T21:21:50Z

Coverage remained the same when pulling cc76e8a on vighneshbirodkar:blob_example into 7a1213a on scikit-image:master.

cdeil · 2014-04-27T21:36:58Z

@vighneshbirodkar OK, the current way you handle the blobs is perfectly fine!
But if e.g. detection methods are introduced that return for some reason multiple model parameters for each blob your tuple unpacking will not work either and a re-formatting into a uniform container before plotting as e.g. here would be the cleanest solution.
If you want to save two lines you could remove the different colors for the blobs since they have no information, but that is a very nitpicky comment.

@stefanv In my opinion this is ready to merge.

cdeil · 2014-04-30T21:25:03Z

@stefanv Merge this now? Can this still go in the upcoming release?

Blob detection example

stefanv · 2014-05-01T10:40:44Z

Thanks, @vighneshbirodkar & @cdeil!

cdeil · 2014-11-07T08:20:49Z

@vighneshbirodkar I don't know if you saw it ... this data example and blob detection is featured in this article on the IPython notebook in Nature (click on the interactive demo link):
http://www.nature.com/news/interactive-notebooks-sharing-the-code-1.16261

Pretty cool!

jni · 2014-11-07T11:22:21Z

👍 !

vighneshbirodkar · 2014-11-08T18:18:31Z

Thanks. I wasn't aware .
On 7 Nov 2014 17:07, "Juan Nunez-Iglesias" notifications@github.com wrote:

[image: 👍] !

—
Reply to this email directly or view it on GitHub
#986 (comment)
.

cdeil reviewed Apr 22, 2014
View reviewed changes

stefanv reviewed Apr 22, 2014
View reviewed changes

stefanv reviewed Apr 26, 2014
View reviewed changes

vighneshbirodkar added 2 commits April 27, 2014 11:49

Added Hubble Deep Field image.

d76aad9

Added example and pep8 changes.

57bd84e

cdeil reviewed Apr 27, 2014
View reviewed changes

vighneshbirodkar added 3 commits April 28, 2014 00:05

Changed example and hubble_deep_field doc string

a82d814

autopep8 changes.

5b459a8

Corrected comment in example.

580f5d2

vighneshbirodkar reviewed Apr 27, 2014
View reviewed changes

Added method reference to example.

cc76e8a

stefanv added a commit that referenced this pull request May 1, 2014

Merge pull request #986 from vighneshbirodkar/blob_example

5e4bce3

Blob detection example

stefanv merged commit 5e4bce3 into scikit-image:master May 1, 2014


		blobs = blob_log(image_gray, max_sigma=30, num_sigma=10, threshold=.1)

		fig = plt.figure()


		blobs = blob_doh(image_gray, max_sigma=30, threshold=.01)

		fig = plt.figure()

Blob Detection Example #986

Blob Detection Example #986

Conversation

vighneshbirodkar commented Apr 22, 2014

vighneshbirodkar commented Apr 22, 2014

cdeil commented Apr 22, 2014

cdeil Apr 22, 2014

Choose a reason for hiding this comment

vighneshbirodkar Apr 22, 2014

Choose a reason for hiding this comment

stefanv Apr 26, 2014

Choose a reason for hiding this comment

vighneshbirodkar commented Apr 22, 2014

cdeil commented Apr 22, 2014

stefanv commented Apr 22, 2014

stefanv commented Apr 22, 2014

vighneshbirodkar commented Apr 22, 2014

stefanv commented Apr 22, 2014

stefanv Apr 22, 2014

Choose a reason for hiding this comment

JDWarner commented Apr 22, 2014

vighneshbirodkar commented Apr 23, 2014

tacaswell commented Apr 23, 2014

stefanv commented Apr 25, 2014

jni commented Apr 25, 2014

stefanv commented Apr 25, 2014

vighneshbirodkar commented Apr 26, 2014

stefanv commented Apr 26, 2014

coveralls commented Apr 26, 2014

stefanv Apr 26, 2014

Choose a reason for hiding this comment

coveralls commented Apr 26, 2014

stefanv commented Apr 27, 2014

cdeil commented Apr 27, 2014

cdeil Apr 27, 2014

Choose a reason for hiding this comment

cdeil commented Apr 27, 2014

stefanv commented Apr 27, 2014

vighneshbirodkar Apr 27, 2014

Choose a reason for hiding this comment

coveralls commented Apr 27, 2014

vighneshbirodkar commented Apr 27, 2014

cdeil commented Apr 27, 2014

vighneshbirodkar commented Apr 27, 2014

cdeil commented Apr 27, 2014

vighneshbirodkar commented Apr 27, 2014

coveralls commented Apr 27, 2014

cdeil commented Apr 27, 2014

cdeil commented Apr 30, 2014

stefanv commented May 1, 2014

cdeil commented Nov 7, 2014

jni commented Nov 7, 2014

vighneshbirodkar commented Nov 8, 2014