Skip to content

New pixel graph implementation - #143

Merged
jni merged 13 commits into
masterfrom
new-pixel-graph
Dec 12, 2021
Merged

jni merged 13 commits into
masterfrom
new-pixel-graph

Conversation

@jni

@jni jni commented Nov 30, 2021

Copy link
Copy Markdown
Owner

This is a major change in skan that removes old junction node handling in favour of the new MST junction "cleanup". Building the graph is much faster and we preserve pixel-perfect correspondence between the skeleton graph and the original image.

  • Use new pure Python method to generate pixel graph
  • Remove JunctionModes and unique_junctions args
  • Remove unnecessary junction mode tests; fix tinycycle test
  • Use new tuple coordinates in draw tests
  • Update pipe to use Skeleton class
  • Update skeleton class testing

@GenevieveBuckley

Copy link
Copy Markdown
Collaborator

Tests are failing because skan isn't able to import a private function from scikit-image. I suspect that the CI is using a newer version of scikit-image where the location of this private utility function has moved.

  File "/opt/hostedtoolcache/Python/3.9.9/x64/lib/python3.9/site-packages/skan/csr.py", line 6, in <module>
    from skimage.morphology._util import _raveled_offsets_and_distances
ImportError: cannot import name '_raveled_offsets_and_distances' from 'skimage.morphology._util' (/opt/hostedtoolcache/Python/3.9.9/x64/lib/python3.9/site-packages/skimage/morphology/_util.py)

@GenevieveBuckley

Copy link
Copy Markdown
Collaborator

Hm, maybe that's not actually the reason. The _raveled_offsets_and_distances function is in the expected place in the scikit-image main branch.

@GenevieveBuckley

Copy link
Copy Markdown
Collaborator

Turns out it's the other way around. The function _raveled_offsets_and_distances was introduced very recently in scikit-image/scikit-image#5602, so currently it is only available from scikit-image version 0.19.0rc0.

So this PR should also update the requirements to specify scikit-image>=0.19.0 (and we'll need to wait a little bit for that release to become available). Sorry Juan, I think you knew all of this already!

@jni

jni commented Dec 2, 2021

Copy link
Copy Markdown
Owner Author

Thanks for reminding me @GenevieveBuckley! tbh I was writing this a while back and I would have had to go figure it out 😂 Probably I'll just vendor the function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants