Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(grainstats): GrainStats.get_max_min_feret() > measure.feret #823

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

ns-rse
Copy link
Collaborator

@ns-rse ns-rse commented Apr 15, 2024

Closes #798

Replaces the functionality for calculating minimum and maximum feret distances within the GrainStats class with the new topostats.measure.feret.min_max_feret().

  • Reorder dictionary returned by min_max_feret().
  • Dictionary is included in the stats dictionary built by GrainStats and saved as CSV.
  • Updates tests/test_grainstats_minicircle.py::test_grainstats_regression to include the co-ordinates.
  • Updates tests/test_grainstats.py::test_process_scan* to include co-ordinates.
  • Removes GrainStats.get_max_min_feret() and associated methods that are called as well as their tests.
  • Co-ordinates for min/max feret are included by default in HDF5 output now that they are part of the returned dataframe.

Will have to rethink/change how results are returned by GraintStats class if height profiles are to be extracted. Probably most sensible to have profiling as a separate step conducted after GrainStats.

Closes #798

Replaces the functionality for calculating minimum and maximum feret distances within the `GrainStats`  class with the
new `topostats.measure.feret.min_max_feret()`.

+ Reorder dictionary returned by `min_max_feret()`.
+ Dictionary is included in the `stats` dictionary built by `GrainStats` and saved as CSV.
+ Updates `tests/test_grainstats_minicircle.py::test_grainstats_regression` to include the co-ordinates.
+ Updates `tests/test_grainstats.py::test_process_scan*` to include co-ordinates.
+ Removes `GrainStats.get_max_min_feret()` and associated methods that are called as well as their tests.
+ Co-ordinates for min/max feret are included by default in HDF5 output now that they are part of the returned
  dataframe.

Will have to rethink/change how results are returned by `GraintStats` class if height profiles are to be
extracted. Probably most sensible to have profiling as a separate step conducted after GrainStats.
Closes #798

Replaces the functionality for calculating minimum and maximum feret distances within the `GrainStats`  class with the
new `topostats.measure.feret.min_max_feret()`.

+ Reorder dictionary returned by `min_max_feret()`.
+ Dictionary is included in the `stats` dictionary built by `GrainStats` and saved as CSV.
+ Updates `tests/test_grainstats_minicircle.py::test_grainstats_regression` to include the co-ordinates.
+ Updates `tests/test_grainstats.py::test_process_scan*` to include co-ordinates.
+ Removes `GrainStats.get_max_min_feret()` and associated methods that are called as well as their tests.
+ Co-ordinates for min/max feret are included by default in HDF5 output now that they are part of the returned
  dataframe. These are rounded to 13 decimal places to address precision errors encountered on Windows machines running
  the test suite.

Will have to rethink/change how results are returned by `GraintStats` class if height profiles are to be
extracted. Probably most sensible to have profiling as a separate step conducted after GrainStats.
Closes #798

Replaces the functionality for calculating minimum and maximum feret distances within the `GrainStats`  class with the
new `topostats.measure.feret.min_max_feret()`.

+ Reorder dictionary returned by `min_max_feret()`.
+ Dictionary is included in the `stats` dictionary built by `GrainStats` and saved as CSV.
+ Updates `tests/test_grainstats_minicircle.py::test_grainstats_regression` to include the co-ordinates.
+ Updates `tests/test_grainstats.py::test_process_scan*` to include co-ordinates.
+ Removes `GrainStats.get_max_min_feret()` and associated methods that are called as well as their tests.
+ Co-ordinates for min/max feret are included by default in HDF5 output now that they are part of the returned
  dataframe. These are rounded to 13 decimal places to address precision errors encountered on Windows machines running
  the test suite.

Will have to rethink/change how results are returned by `GraintStats` class if height profiles are to be
extracted. Probably most sensible to have profiling as a separate step conducted after GrainStats.
@ns-rse ns-rse marked this pull request as ready for review April 15, 2024 09:57
Copy link
Collaborator

@MaxGamill-Sheffield MaxGamill-Sheffield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good:

  • ferets match previous ferets
  • coords returned in grainstats.csv save re-calculating this later

@MaxGamill-Sheffield MaxGamill-Sheffield added this pull request to the merge queue Apr 17, 2024
Merged via the queue into main with commit e7ff193 Apr 17, 2024
14 checks passed
@MaxGamill-Sheffield MaxGamill-Sheffield deleted the ns-rse/748-grain-profiles-grainstats branch April 17, 2024 09:27
@ns-rse
Copy link
Collaborator Author

ns-rse commented Apr 17, 2024

Consider excluding co-ordinates from GrainStats output.

ns-rse added a commit that referenced this pull request Apr 17, 2024
Further to #823 this reverts the GrainStats output to its previous form and removes the feret co-ordinates from the
output as discussed in this mornings meeting.
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.

Look at replacing feret and centroid calculations in GrainStats
2 participants