Shape contexts

[sivapvarma]

Shape Contexts

paper link: http://www.eecs.berkeley.edu/Research/Projects/CS/vision/shape/belongie-pami02.pdf

refactor of code from PR#235 to make it more efficient and documentation

Current API :

import numpy as np
from skimage.feature import descriptor_shapecontext
img = np.zeros((20, 20)).astype(float)
img[4:8, 4:8] = 1
px = (10, 10)
print descriptor_shapecontext(img, 0, 25, px, radial_bins=3, polar_bins=4)

[sciunto]

Please, write int instead of integer
Here, write "int, optional"

[sivapvarma]

Update: build now passes :-)

[sciunto]

Could you put the default value in the line below please?

[sciunto]

Cool. I just scrooled the paper, it looks really interesting. Is it possible to make an example for the gallery? (see doc/examples)

Core dev may have other comments. I have not read the code in details yet :)

[sivapvarma]

@sciunto I want to add the MNIST classification results of the paper which was the best on that dataset at the time the paper was published in 2002. They still are the best features with kNN classifier on MNIST. I may need to use sklearn.neighbors.KNeighborsClassifier and the pre-processing required to get 0.63 classification rate on MNIST is a bit involved. So it'll mostly become a lengthy demonstration than an example.

[RONNCC]

spelling error

[RONNCC]

also +1 for the gallery addition, this is a cool paper!

[stefanv]

Perhaps a better place for such a demo would be
https://github.com/stefanv/scikit-image-demos.git for now. I could move
that repo to the scikit-image org if it is of more general interest.

[sciunto]

[3] ?

[RONNCC]

ehh yeah i was surprised too, but I guess it can't hurt if he learned what a shape context is there...

[sivapvarma]

I just included it for completeness since there is a wiki page.

[RONNCC]

so in general we only cite the paper + if there's relevant source paper + if you looked at any code for reference but usually just the paper,

[sivapvarma]

then i'll remove that reference

[jni]

@stefanv I'd agree with that new home --- do we have any examples that use sklearn in the gallery? I'm guessing we don't want to introduce that dependency?

[jni]

Actually, sorry @sivapvarma for conflicting advice, but we only put the default values in the docstring for Cython functions, where there is no way to discover them by introspection. For Python functions, we don't specify default values, as they can be guessed independently, and putting them in the docstring increases the likelihood that the docstring and the definition will get out of sync.

@sciunto correct? Or am I missing something?

We do, however, start our docstring lines with a capital and end them with a full stop .. ;)

[RONNCC]

@jni nope that sounds correct, other projects do this too, the default value still shows up in the function declaration + args when the docs are generated, so having them twice is redundant

[sivapvarma]

@stefanv @jni I might be wrong but isn't doc/examples/applications on trunk currently intended to hold longer examples and demos.

[jni]

@sivapvarma Yes, but notice that the examples there don't add any dependencies.

[stefanv]

Wikipedia citations are okay, as long as the whole thing is given: title + url (also, I prefer to only use it if the article is actually any good)

[stefanv]

https://github.com/scikit-image/scikit-image-demos

[sivapvarma]

I added an example. The images are from MNIST handwritten digits dataset.

[sivapvarma]

I think the API can be improved a bit. descriptor_shapecontext() is a bit superfluous. shapecontext() should be enough. Also currently shapecontext.py doesn't do much, so I want to remove it, and let _shapecontext.pyx do everything.

If everybody agrees, I'll go ahead and make the changes

[sciunto]

@jni well, OK.

[ahojnnes]

We should keep the Python wrapper.

Also, to comply with the rest of the feature detectors / descriptors (e.g. BRIEF, ORB), I suggest to wrap it in a class. I know it's not necessary in case of shapecontext, but we should aim for consistency. In the long run we should also discuss if we want to change HOG and Daisy top the new interface.

But let's first discuss this.

[jni]

@ahojnnes I'm -1 on adding classes when a functional interface works equally well. To be honest I'm still not clear on why BRIEF and ORB weren't working as functions...

[stefanv]

Spell check

[hmaarrfk]

@sivapvarma, do you know how to update your master branch and rebase onto that?

If not, we might be able to help get this ready if you "enable commits from maintainers" on the bottom of the right side bar.

Ultimately, we can always fork off your branch, but I feel there is more satisfaction if you get your orignal PR accepted start to finish 😄

[pep8speaks]

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

• In the file doc/examples/plot_shapecontexts.py:

Line 23:80: E501 line too long (87 > 79 characters)
Line 27:80: E501 line too long (93 > 79 characters)

• In the file skimage/feature/__init__.py:

Line 26:3: E227 missing whitespace around bitwise or shift operator
Line 26:7: E225 missing whitespace around operator
Line 30:1: E402 module level import not at top of file
Line 31:3: E225 missing whitespace around operator
Line 31:5: E225 missing whitespace around operator
Line 31:7: E225 missing whitespace around operator
Line 32:1: E402 module level import not at top of file
Line 33:1: E402 module level import not at top of file
Line 34:7: E225 missing whitespace around operator

• In the file skimage/feature/setup.py:

Line 15:1: E305 expected 2 blank lines after class or function definition, found 1
Line 15:3: E227 missing whitespace around bitwise or shift operator
Line 15:7: E225 missing whitespace around operator
Line 32:3: E225 missing whitespace around operator
Line 32:5: E225 missing whitespace around operator
Line 32:7: E225 missing whitespace around operator
Line 33:5: E113 unexpected indentation
Line 40:7: E225 missing whitespace around operator

• In the file skimage/feature/shapecontext.py:

Line 34:80: E501 line too long (89 > 79 characters)

• In the file skimage/feature/tests/test_shapecontext.py:

Line 8:5: E741 ambiguous variable name 'I'
Line 17:5: E741 ambiguous variable name 'I'
Line 19:5: E741 ambiguous variable name 'I'

[sivapvarma]

@hmaarrfk I rebased on to master and also allowed edits from maintainers.

Let me know if there is anything else.