Stoichiometry recruitment

[sallymatson]

Description

This PR creates an add_cohorts method for Stochiometry, which allows us to add new cohorts to match David's recruitement PR #827.

When we merged in Stochiometry, this caused the tests to fail for that PR because while community cohorts were being added nothing was happening for stochiometry, leading to dimension mismatches.

This PR should fix that issue.

I am not convinced this is the cleanest code in the world but I really wanted to get in a draft before going on leave. @davidorme, completely up to you how to proceed - if you have major comments and want to wait for me to get back, I'll do it asap when I return. Otherwise, feel free to modulate on this code and then merge it in so you can get recruitement fully off your plate!! Unfortunately, recruitement cannot be merged without this kind of (relatively major) fix. So I hope this is helpful.

If you decide to merge with outstanding issues, feel free to add as todos for me to finish when I get back.

Next step will be making a similar method for mortality :)

Type of change

• [x ] New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist

• [ x] Make sure you've run the pre-commit checks: $ pre-commit run -a
• [ x] All tests pass: $ poetry run pytest

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works
• Relevant documentation reviewed and updated

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (824-recruitment-of-new-tree-cohorts@9a49ea0). Learn more about missing BASE report.

Additional details and impacted files

@@                          Coverage Diff                           @@
##             824-recruitment-of-new-tree-cohorts     #971   +/-   ##
======================================================================
  Coverage                                       ?   94.29%           
======================================================================
  Files                                          ?       79           
  Lines                                          ?     6612           
  Branches                                       ?        0           
======================================================================
  Hits                                           ?     6235           
  Misses                                         ?      377           
  Partials                                       ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[davidorme]

This looks good.

I'd kinda like to be able to reuse the pyrealm CohortMethods mixin here, but I think that requires serious structural changes to work cleanly - essentially we'd need to separate the elements within tissues. That might be worth doing in the long run - we could have dict[str, Element] within a tissue and then Element.ideal_ratio etc. But - that's a thing to note and park.

In the short term, I think I'd prefer the tissue classes to have their own add_cohort methods as a new abstract method of the Tissue ABC. It's a thing a new tissue has to have and if we define it there, then the section in the StemStochiometry.add_cohorts() will automatically handle new tissues.

[davidorme]

I meant to include a suggestion showing what I meant, but clicked submit review too early.

Another thing that has just occurred to me: I think StemStochiometry.add_cohorts should loop over the elements internally? We'd never want to just add data on one element, so having to call it multiple times to make sure each element is handled doesn't make sense.

If this is enforced by an outer loop in the calling code, then we might want to revisit the looping over elements structure. That might fit in with the comment above about having an explicit Element class, so might be a rework for later, but its definitely something that bothers me!

[davidorme]

LGTM.