Skip to content

Include node count in grain_statistics.csv#1382

Merged
tobyallwood merged 7 commits into
mainfrom
tobyallwood/node-count
Jul 16, 2026
Merged

Include node count in grain_statistics.csv#1382
tobyallwood merged 7 commits into
mainfrom
tobyallwood/node-count

Conversation

@tobyallwood

@tobyallwood tobyallwood commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Add node count to grain stats

Closes #1379

After nodestats has run the total number of nodes can be collected and added to the grain's stats dictionary (called num_nodes)

The most appropriate .csv to save to is grain_statistics.csv, but there was an issue with how we'd do this as nodestats are collected on a graincrop level but GrainCrop.stats (therefore also grain_statistics.csv) is further divided into class and subgrain. For now I have just duplicated the num_nodes value across all rows related to a grain - not a perfect solution but better than having empty cells that could cause confusion.

A small bug has also been fixed related to the missing disordered tracing stats that were recently re-added where not running disordered tracing would cause a KeyError. This has been rectified with a quick check before attempting to access.


  • Existing tests pass.
  • Documentation has been updated and builds. Remember to update as required...
    • docs/usage/data_dictionary.md
  • Pre-commit checks pass.

@tobyallwood
tobyallwood requested a review from ubdbra001 July 15, 2026 10:50

@ubdbra001 ubdbra001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not super happy with the nested loops, but it looks like the usual way of doing things, so I can give it a pass.

I'd recommend adding test cases for highlight_node_centres at least. There is a stub already, but now would be a good time to complete it.

@SylviaWhittle

Copy link
Copy Markdown
Collaborator

Calculated by count of labels, this doesn’t do what was done before where we counted simply the number of pixels in the node mask, which returned an erroneously high number of nodes for a given image.

@tobyallwood

tobyallwood commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

I've added a test for highlight_node_centres, and from this some commented out fixtures in tests/tracing/conftest.py have been brought back.

I've also added __init__.pys for both /tests/ and /tests/tracing/ as not having these was causing pylint to throw an error and fail.

@tobyallwood
tobyallwood requested a review from ubdbra001 July 16, 2026 11:07

@ubdbra001 ubdbra001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be good if you could include tests for no nodes and multiple nodes if possible, just so the behaviour of the function is explicit and tested for these cases too.
Though that's more of a should than a must, so if it's not something that could be added in (max) an afternoon then just add a note to the test that these cases need to be added later.

Once either of the above is done then this looks good to go.

@ubdbra001 ubdbra001 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great, ready to merge!

Thanks!

If disordered tracing runs but one/ some of the grains fail (meaning they have no disordered trace) this will now be checked before attempting to access this information
@tobyallwood
tobyallwood added this pull request to the merge queue Jul 16, 2026
Merged via the queue into main with commit acf1317 Jul 16, 2026
7 checks passed
@tobyallwood
tobyallwood deleted the tobyallwood/node-count branch July 16, 2026 14:12
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.

[feature] : Re-add correct node count

3 participants