Skip to content

Conversation

@tobyallwood
Copy link
Collaborator

@tobyallwood tobyallwood commented Oct 29, 2025

TopoStats Pull Requests

Merge conflicts from tobyallwood/threshold-data-types attempting to merge into the most recent version of main have been resolved and all tests pass.

In this change:

  • GrainCropDirections and ImageGrainCrops classes have been removed.
  • crops dict and full_mask_tensor arrays have been moved to the TopoStats class (where ImageGrainCrops used to be stored)
  • Directions ('above', 'below') have been removed, now grain threshold indexes (of those given in config files) are used and corresponding grain thresholds for each grain are stored in the GrainCrop class instances.
  • Data type for thresholds entered in config files has been changed from dicts (keys 'above' and 'below') to lists with positive and negative numbers denoting above or below thresholds.
  • Tests updated to reflect these changes

Before submitting a Pull Request please check the following.

  • Existing tests pass.
  • Documentation has been updated and builds. Remember to update as required...
    • docs/configuration.md
    • docs/usage.md
    • docs/data_dictionary.md
    • docs/advanced.md and new pages it should link to.
  • Pre-commit checks pass.
  • New functions/methods have typehints and docstrings.
  • New functions/methods have tests which check the intended behaviour is correct.

topostats/default_config.yaml

If adding options to topostats/default_config.yaml please ensure.

  • There is a comment adjacent to the option explaining what it is and the valid values.
  • A check is made in topostats/validation.py to ensure entries are valid.
  • Add the option to the relevant sub-parser in topostats/entry_point.py.

@ns-rse
Copy link
Collaborator

ns-rse commented Oct 30, 2025

Hi @tobyallwood

Good work going through all the conflicts. Does this make #1213 redundant now then? If so it can be closed.

We should also update the title of this Pull Request to reflect the work it contains (removal of GrainCropsDirection and ImageGrainCrops) as we want the commit history to reflect that (resolving merge conflicts is just an artefact of using Git).

@tobyallwood tobyallwood changed the title Merge conflicts from tobyallwood/threshold-data-types and main resolved Removal of GrainCropDirections and ImageGrainCrops classes Oct 30, 2025
@tobyallwood
Copy link
Collaborator Author

tobyallwood commented Oct 30, 2025

Hi @ns-rse yep this is #1213 with merge conflicts sorted, and have updated the title 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest disabling too-many-positional-arguments once at the top for the whole file.

We don't really care about this particular linting rule for tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorted

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.

3 participants