300 add connection to meorg client to successful run #312
Conversation
Updated hpcpy version.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #312 +/- ##
=======================================
Coverage 68.28% 68.28%
=======================================
Files 21 21
Lines 1157 1157
=======================================
Hits 790 790
Misses 367 367 ☔ View full report in Codecov by Sentry. |
|
Following some version issues on the hpcpy end, I think this is ready for review. |
src/benchcab/utils/meorg.py
Outdated
|
|
||
| logger = bu.get_logger() | ||
|
|
||
| model_output_id = config.get("fluxsite").get("meorg_model_output_id", False) |
There was a problem hiding this comment.
Please replace get with regular dictionary indexing, default values in the config should be initialised here
There was a problem hiding this comment.
Done, although I think it would be good practice to use the .get approach in development with a default for the null case. Perhaps we could override the get method to do a lookup in place of a missing key rather than manually unpacking everything as it currently is in internal.py
I think we should flag this reliance on internal.py to set up defaults for the portability project. It seems to me that we could essentially configure all defaults with the schema itself...
|
OK, so I added in the review comments and this spurred a deluge of changes required over the tests. I also took the opportunity to run black/ruff over the whole lot to bring the formatting up to scratch. In doing so, I think we really need to address those config tests - there is a lot of hardcoding and very limited visibility on what breaks where in the assertions. I would suggest we make the changes and then render back to yaml and compare results to expectations. At least this way we can get an explicit line/character where the comparison fails rather than somewhere in the |
Copy/paste error.
|
OK @SeanBryan51 , what is going on here? I didn't touch anything with setuptools. https://github.com/CABLE-LSM/benchcab/actions/runs/11246285162/job/31267853382?pr=312#step:4:388 |
|
@bschroeter the CI issue looks like a breaking change with newly released python version: see #315 |
Update 300 from main.
|
CI updated from main, pytest passing now. |
|
@SeanBryan51, bumped python version to <3.13 |
bschroeter
deleted the
300-add-connection-to-meorg_client-to-successful-run
branch
Code for the integration work.