[WIP] Get maximum branch #101
Conversation
There was a problem hiding this comment.
@snowformatics thanks for this PR! I've made some comments that I hope will help you get a bit further in!
skan/csr.py
Outdated
| The maximum length in the skeleton. | ||
| """ | ||
|
|
||
| sums = np.maximum.reduceat(self.paths.data, self.paths.indptr[:-1]) |
There was a problem hiding this comment.
This should be called maxes, obviously. ;)
There was a problem hiding this comment.
Sorry, I still don't get what you mean :( Do you mean the ufunc maximum? Or the docstring naming?
There was a problem hiding this comment.
I think Juan means that he would prefer the variable sums instead be called maxes (because it was arrived at using the numpy maximum function, I imagine)
There was a problem hiding this comment.
Ok thanks, I missunderstood.
| # 11-14 --> [1. 1. 1. ] --> maximum --> 1. | ||
| # 14-17 --> [1. 1. 1. ] --> maximum --> 1. | ||
| # 17: --> [1. 1. 1. 1.] --> maximum --> 1. | ||
| # which will end up with sums = [1. 1. 1. 1.] |
There was a problem hiding this comment.
Yeah, so the problem is that the test data is not "interesting" enough for this problem. To make it more interesting, you should change the test data to be e.g. random numbers. Or whatever, like a pyramid, [1, 2, 1], [1, 2, 2, 1], [1, 2, 3, 2, 1]...?
Then, you can just return the maxes! We are only after the maximum value, right?
The key is that when you replace the values of the skeleton by the distance transform, rather than just 1s, getting the max will tell you the maximum width of the skeleton at each branch.
On the other hand, maybe what you want is the minimum width? Or the width at the midpoint of the branch? If you think about how a skeleton image tends to look, the junctions are almost always farther away from the edge of the object than the middle of the branches.
Sorry, I'm about to hop in bed, I hope that this makes sense, if not I will try to draw a diagram tomorrow, sometimes text is not enough. 😊
There was a problem hiding this comment.
I see, I will try and come back again :) Thanks
There was a problem hiding this comment.
@jni perhaps you should try and clarify this (@snowformatics did say below they aren't sure how to follow your testing suggestions). This'll be the most important next step to unstick this PR
| print (csr.path_maximum_length()) | ||
| print (stats) | ||
| #print (stats.loc[0, 'max-length'], expected) | ||
| #assert_almost_equal(stats.loc[0, 'max-length'], expected) |
There was a problem hiding this comment.
As above: you need to make a slightly more interesting test case for this, just by varying the values inside the skeleton.
|
Hi @snowformatics, Are you still working on this by any chance? 😃 |
Hi @mkcor , unfortunately not but would be still interested to work on it with some support. I was kind of stucked (my first PR) Cheers |
Great! How can I help you?
|
Co-authored-by: Marianne Corvellec <[email protected]>
Co-authored-by: Marianne Corvellec <[email protected]>
Help would be great, thanks a lot! |
|
Hi @snowformatics,
Either use the online editor (by clicking the "Resolve conflicts" button in this page), or use your own editor locally (after merging current
Basically, Git needs to know which changes you want to include exactly. Unless you want to use
Once you are done (file looks the way you intend), commit and you're done! If you were editing online, don't forget to pull, and if you were working locally, don't forget to push this update. |
This was very helpful, thanks for your patience! I hope it's correct now. |
|
Great job, @snowformatics! You're welcome. As per testing, I can see that you started with a 1D image: image = np.random.random((45,))Following https://jni.github.io/skan/getting_started.html#extracting-a-skeleton-from-an-image, do I understand correctly:
? I think that a 2D toy example would be nice as well, so we can view it with imshow :) |
Trying to implement a maximum branch method. Currently not working but with questions as comments .