metoppv / improver

IMPROVER is a library of algorithms for meteorological post-processing.
http://improver.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
103 stars 85 forks source link

Migrate CLI functionality to plugin layer subset 2 #2003

Closed cpelley closed 2 months ago

cpelley commented 4 months ago

Second/final chunk of plugin changes required by EPP.

Tasks

Issues

cpelley commented 3 months ago

Finally got this ready for review. This covers the second and final chunk of IMPROVER CLI/plugin changes required specifically for EPP. Cheers

cpelley commented 3 months ago

@bayliffe, directly relevant change in reflection of this work in https://github.com/MetOffice/dagrunner/pull/40 (you need not review that, I mention only out of interest).

cpelley commented 3 months ago

Changes based on feedback from @bayliffe:

The only thing I haven't done as I'm not quite following is what the issue is, is in regards to https://github.com/metoppv/improver/pull/2003#discussion_r1665628061 However, I responded to what I think is a misunderstanding. *args is being passed to the function. The idea is just 1 parameter could be passed if its a cubelist, or 2 or 3 etc. I have verified that the documentation does highlight what data is actually needed to be provided.

cpelley commented 3 months ago

You happy to approve @bayliffe?

cheers

cpelley commented 3 months ago

I suspect the CI failures with Sphinx are unrelated to this PR (a result of changes in the conda-forge conda environment, prob. needs something pinning or something unpinning). Fix for CI in https://github.com/metoppv/improver/pull/2011

cpelley commented 3 months ago

I have merged in master into this branch and changed the target (since subset 1 was merged I forgot to change it here).

cpelley commented 3 months ago

@mo-robert-purvis, you mind approving once more (it removed your approval after I changed the target to master). cheers

cpelley commented 2 months ago

I just merged master into this branch to resolve the CI failure.

cpelley commented 2 months ago

Thanks both. @bayliffe, I have made the leadtimes optional once more. Thanks for double checking this for me.