Prep for lstbin subpackage

[steven-murray]

Adds a new "type" of makeflow creator, for the LSTbin-notebook.

[codecov]

Codecov Report

Attention: Patch coverage is 93.05556% with 5 lines in your changes missing coverage. Please review.

Project coverage is 98.87%. Comparing base (c73a533) to head (834c974).

Files	Patch %	Lines
hera_opm/mf_tools.py	93.05%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #150      +/-   ##
==========================================
- Coverage   99.46%   98.87%   -0.59%     
==========================================
  Files           4        4              
  Lines         742      710      -32     
==========================================
- Hits          738      702      -36     
- Misses          4        8       +4     

Flag	Coverage Δ
unittests	98.87% <93.05%> (-0.59%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[plaplant]

Thanks very much @steven-murray! This looks great.

One general comment I have is that I'm a little worried about the large dip in test coverage. I know that sometimes it can't be helped, especially if it depends on having different options in the config files, but this seems like a significant number of lines that are not covered by tests (almost 100 lines by codecov's reckoning). How hard would it be to increase the coverage for some of these lines?

If it's not easy to increase coverage, and we've tested that these new functions work "in the wild", then we can merge as is. Just checking to see how big a lift it would be.

[steven-murray]

@plaplant thanks for the review! Yes, I noticed the same thing, and I'm trying to improve test coverage now. The biggest problem is that there are now three ways of creating the LST-bin makeflows: the <=H4C way, the legacy H6C way, and the new lstbin-notebook way. Some of these depend on different versions of hera_cal being installed. So, it will take a bit of effort to make sure all of them are tested at once. I will give it a go.

[jsdillon]

Would it be a lot less work to just drop the "legacy H6C" way?

[steven-murray]

@plaplant OK I thought through it a bit and realized that I was just trying to support too many different iterations of hera_cal. I've updated it so that it now only supports the newer interface -- if we need to re-run older makeflows, we will just need to install older versions of hera_opm, which should be perfectly easy to do. I also realized that with the new configuration in hera_cal, the creation of notebook-based and script-based makeflows is very similar, so I merged them into one function. Coverage is back up to very high.

If you think we need to maintain support for older analysis configurations, then I can try putting those back in.

[jsdillon]

What's the status of this PR? Apparently, it's necessary for the new LST-binner (and therefore H6C analysis).

[steven-murray]

@plaplant this is now ready I think. the code coverage is not 100% but the lines missed are mostly exceptions for corner cases that don't occur naturally.

[jsdillon]

Is this ready to go @plaplant ? I'd love to stop using a branch for H6C IDR3.2

[plaplant]

Thanks @steven-murray! I just have a small comment, but then I'm happy to merge this 👍