rename feret_diameter -> max_feret_diameter ?
#4817
Labels
Comments
|
A good point! I'd be 👍 on this. (And yes it's totally fine to change this pre-release.) |
sciunto
added
⚠️ Critical
|
I'm fine with this! |
|
thanks for considering this. Then I need to tidy up the code I have for minimum feret diameter (unfortunately only 2D) next. Also saw that @maxfrei750 already created a pull request. Not sure whether that'll need rebasing now that the new custom regionprop feature has been merged. |
Is your code for the minimum Feret diameter already available in scikit-image or somewhere else? |
|
Hi @maxfrei750 , |
|
@maxfrei750 (apologies for the delay) and others who are interested: Here is a gist https://gist.github.com/VolkerH/0d07d05d5cb189b56362e8ee41882abf that contains code for calculating minimum feret diameters in the 2D case. This would still need a bit of tidying before it could go into scikit image and I currently don't have the time. What needs tidying?
I a probably should open a new issue for this, but then I feel I have to work on it. |
ns-rse
added a commit
to AFM-SPM/TopoStats
that referenced
this issue
Dec 11, 2023
New submodule `measures.feret` with functions for calculation of min and max feret and the associated co-ordinates. Utilises code from Skan developer see [gist](https://gist.github.com/VolkerH/0d07d05d5cb189b56362e8ee41882abf) and as [suggested](scikit-image/scikit-image#4817 (comment)) it adds tests. In doing so I found sorting points prior to calculation of upper and lower convex hulls missed some points. I'm also not sure about the `rotating_calipers()` function as it doesn't seem to return all pairs formed across the points from the upper and lower convex hulls and so have included but not used the `all_pairs()` function which does, although if used in place this doesn't return the minimum feret correctly, rather just the smallest distance. Currently some of the tests for the `curved_line` fail too. The intention is to use this without instantiating the `GrainStats` class to be used in profiles (#748) and could serve as a concise stand-alone set of functions outside of `GrainStats` class which currently has static methods for the calculations (#750). Benchmarking In light of #750 and re-implementing feret calculations as a new sub-module I wanted to see how this compares in terms of performance to those in `GrainStats()` class so have run some very basic benchmarking. Note this is not the full pipeline of taking labelled images and finding the outlines/edge points which are required as inputs to the calculations, its purely on the calculation of min-max feret from the edge points. ``` import timeit import numpy as np from skimage import draw from topostats.measure import feret from topostats.grainstats import GrainStats holo_circle = np.zeros((14, 14), dtype=np.uint8) rr, cc = draw.circle_perimeter(6, 6, 5) holo_circle[rr, cc] = 1 holo_circle_edge_points = np.argwhere(holo_circle == 1) %timeit feret.min_max_feret(holo_circle_edge_points) 83 µs ± 686 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each) %timeit GrainStats.get_max_min_ferets(holo_circle_edge_points) 1.06 ms ± 10.4 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each) ``` So this new implementation is faster.
ns-rse
added a commit
to AFM-SPM/TopoStats
that referenced
this issue
Dec 11, 2023
New submodule `measures.feret` with functions for calculation of min and max feret and the associated co-ordinates. Utilises code from Skan developer see [gist](https://gist.github.com/VolkerH/0d07d05d5cb189b56362e8ee41882abf) and as [suggested](scikit-image/scikit-image#4817 (comment)) it adds tests. In doing so I found sorting points prior to calculation of upper and lower convex hulls missed some points. I'm also not sure about the `rotating_calipers()` function as it doesn't seem to return all pairs formed across the points from the upper and lower convex hulls and so have included but not used the `all_pairs()` function which does, although if used in place this doesn't return the minimum feret correctly, rather just the smallest distance. Currently some of the tests for the `curved_line` fail too. The intention is to use this without instantiating the `GrainStats` class to be used in profiles (#748) and could serve as a concise stand-alone set of functions outside of `GrainStats` class which currently has static methods for the calculations (#750). Benchmarking In light of #750 and re-implementing feret calculations as a new sub-module I wanted to see how this compares in terms of performance to those in `GrainStats()` class so have run some very basic benchmarking. Note this is not the full pipeline of taking labelled images and finding the outlines/edge points which are required as inputs to the calculations, its purely on the calculation of min-max feret from the edge points. ``` import timeit import numpy as np from skimage import draw from topostats.measure import feret from topostats.grainstats import GrainStats holo_circle = np.zeros((14, 14), dtype=np.uint8) rr, cc = draw.circle_perimeter(6, 6, 5) holo_circle[rr, cc] = 1 holo_circle_edge_points = np.argwhere(holo_circle == 1) %timeit feret.min_max_feret(holo_circle_edge_points) 83 µs ± 686 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each) %timeit GrainStats.get_max_min_ferets(holo_circle_edge_points) 1.06 ms ± 10.4 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each) ``` So this new implementation is faster.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Sorry, I meant to comment on this while it was still an open PR, but then forgot to do so.
As it has not been in a release, maybe it is not too late to rename the new region property
feret_diametertomax_feret_diameter.Tagging @emmanuelle as she added it.
The simple reason being is that sometimes the minimum feret diameter is of interest and
it would be nice to be able to have naming consistency so that a
min_feret_diameterpropertycan get added later.
The text was updated successfully, but these errors were encountered: