Move profile_line() out of viewer

[jni]

The profile_line() function is actually quite useful outside of the viewer and interactive LineProfiler plugin. I've moved it to the measure package and imported it from there for use in the plugin.

In addition, I fixed an apparent bug in its functionality: it appeared to return the profile in the wrong direction:

before:

In [1]: x = array([[2, 2, 2, 1, 1, 1]])

In [2]: y = vstack([zeros_like(x), x, x, x, zeros_like(x)])

In [3]: y
Out[3]: 
array([[0, 0, 0, 0, 0, 0],
       [2, 2, 2, 1, 1, 1],
       [2, 2, 2, 1, 1, 1],
       [2, 2, 2, 1, 1, 1],
       [0, 0, 0, 0, 0, 0]])

In [4]: from skimage import measure as m

In [5]: m.profile_line(y, ((1, 2), (5, 2)))
Out[5]: 
array([[ 1.],
       [ 1.],
       [ 2.],
       [ 2.]])

after:

In [15]: m.profile_line(y, ((1, 2), (5, 2)))
Out[15]: 
array([[ 2.],
       [ 2.],
       [ 1.],
       [ 1.]])

It also adds a mode parameter to be passed in to the ndimage.map_coordinates interpolation routine to estimate values outside the image.

In addition, I'd like to request people's opinion about two additional proposed changes.

1. Currently, the API uses x and y as inputs, meaning the horizontal and vertical dimensions, meaning columns and rows, meaning exactly the opposite of typical numpy indexing. Since we use numpy indexing almost everywhere else, would people support changing this to expect numpy-style coordinates? Since it is not currently a public function, I think we could change this without worrying about API changes. In which case, I would actually support the syntax profile_line(img, src_point, dst_point), where each of those is a valid numpy indexing tuple into img.

2. I would also change the return type to be a 1D array when input is a 2D, single channel image, and 2D array when input is a 2D + c image.

Thoughts? @tonysyu, @JDWarner?

[coveralls]

Changes Unknown when pulling e24ae38 on jni:profile-line into * on scikit-image:master*.

[JDWarner]

In theory I approve of both API changes (and then making the function public, it is handy).

I need to play with it a bit to see if I can get it to break - horizontal and vertical lines are challenges for arctan - but it looks good on a first pass!

[ahojnnes]

I haven't looked at the code, but if you struggle with vertical lines, you probably want to use a different parameterization for lines. See e.g. skimage.measure.LineModel.

[stefanv]

@jni My first thought when looking at the code examples was also that we should change the coordinates to row+column.

[stefanv]

+1 for 1D and 2D returns--then it is similar to a specialized slicing operator

[stefanv]

We have Brezenham's algorithm implemented--can we use that instead to find the line coordinates?

Otherwise, in a case like this, the line can be parameterized as start_point * (1 - alpha) + end_point * alpha (linear interpolation).

[tonysyu]

👍 on all these updates. BTW, I didn't actually write this implementation; I believe @blink1073 wrote the original, just in case he has any input.

[jni]

@JDWarner @ahojnnes, the function automatically detects vertical lines and treats them differently. I'll add a vertical test case anyway. =)

@stefanv I'm not sure about other parameterizations, as they might make it more difficult to calculate the perpendicular scan lines? (Slightly out of my depth here.)

[blink1073]

@tonysyu, profile_line predates me. I think there used to be a note in the docstring that said who wrote the original implementation.

[stefanv]

I'm suggesting you use the module in draw, then there's no need for special
treatment.

[jni]

@stefanv, draw.line can't handle width, which is needed here.

[coveralls]

Changes Unknown when pulling 2a6ec20 on jni:profile-line into * on scikit-image:master*.

[stefanv]

This hack won't fly.

[jni]

@stefanv, can you suggest a fix? As I mentioned, the functions in skimage.draw are only a very partial solution, because profile_line() has a linewidth argument and averages across a perpendicular scan line at each point in the line.

As an aside, I don't think it's reasonable to require PRs to address pre-existing hacks (if that's not the point of the PR).

[stefanv]

@jni Apologies if I wasn't clear: I don't think it is your responsibility to fix it, but I'd like for us all to have a closer look at this when you're done. That piece of code bothers me.

[jni]

@stefanv FINE, you win. I don't know why but you always inspire me to do work I don't actually want to do. =P I need to do a little bit more testing, but I think the current implementation is correct!

[coveralls]

Changes Unknown when pulling 3d93582 on jni:profile-line into * on scikit-image:master*.

[coveralls]

Changes Unknown when pulling 3d93582 on jni:profile-line into * on scikit-image:master*.

[jni]

More API questions: the order of the interpolation is important. Currently, it uses the default (cubic spline). However, this is a nightmare for testing, and, in my case at least, I'd rather just use nearest-neighbor or at most linear interpolation. My suggestion is:

• add an order=blah keyword argument to be passed on to ndimage.map_coordinates for interpolation.
• make blah equal to 0 or 1 by default. (I realise that order=0 will result in some aliasing.)
• (optional, what do people think?) call the function with order=3 from the viewer plugin to preserve current behavior.

Please let me know any thoughts. I'm working on the first point now for the test cases, but I'd love to hear your opinions on the second and third points, @stefanv @tonysyu @JDWarner.

[jni]

... Having spent a bit more time with the testing, I now prefer order=1 as the default. It's more intuitive, in the sense that any line across a linear gradient can be expressed using np.arange.

[jni]

Okely: it all works, hackish if statements have been eliminated, and there's a buttload of tests to go with it, which have been manually checked by yours truly (with line profiles in all sorts of directions including horizontals and verticals). The interpolation order is 1 by default, which I think gives the most interpretable results.

As far as I'm concerned this is ready to merge (pending passing the tests), but feel free to prove me wrong! =)

And @stefanv, I learned a lot getting this done, and it looks way better than it used to, so, as usual, thanks for the nudges. =)

[coveralls]

Changes Unknown when pulling 040a21a on jni:profile-line into * on scikit-image:master*.

[jni]

As an added bonus, the LineProfile plugin now returns the line drawing as an overlay and the scan line as data, as per the API introduced in #810.

[coveralls]

Changes Unknown when pulling ec0079d on jni:profile-line into * on scikit-image:master*.

[stefanv]

@jni One just has to plant a seed in fertile ground and watch it grow ;)

[stefanv]

2d -> 2D or 2-D; not sure what 2d + c means

[jni]

This used to say "2D RGB" but it works for any number of channels. I want to distinguish that from a 3D grayscale image, which wouldn't work. Suggestions?

[stefanv]

(M, N) greyscale or (M, N, ...) multi-channel perhaps?

[stefanv]

Love it, thanks @jni !

[jni]

@stefanv, thanks for the comments. =D The seed analogy is pretty good... It sat there and grew and grew in my head as I tried to focus on other stuff. =D

Let me know if you want me to add the width to the returned line. I think the added code complexity wouldn't add much, but am open to adding it.

[stefanv]

Could we perhaps compute the vertices of the box in case of width > 1 and
then use our polygon drawing tool? If not, I'm okay with it as-is.
On 30 Jan 2014 02:19, "Juan Nunez-Iglesias" notifications@github.com
wrote:

@stefanv https://github.com/stefanv, thanks for the comments. =D The
seed analogy is pretty good... It sat there and grew and grew in my head as
I tried to focus on other stuff. =D

Let me know if you want me to add the width to the returned line. I think
the added code complexity wouldn't add much, but am open to adding it.

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

[coveralls]

Changes Unknown when pulling 01967e5 on jni:profile-line into * on scikit-image:master*.

[jni]

How about I create an issue to change this and deal with it in a later PR? I'd like to get on with actually using profile_line() for my research. =)

[stefanv]

Cool, let's get on with it :) Thanks!