ENH add mandatory source parameter to odim writer

[egouden]

Here is a small improvement

[kmuehlbauer]

Thanks @egouden for adding this missing mandatory part.

I'm linking the ODIM_H5 2.2 document for reference.

Below is table 3 of the linked document, showing some description of the possible content.

We might want to check if at least one of the listed descriptors (eg. WMO) is given in the source-kwarg. Just to make sure we really write standard conform metadata.

For the error in CI (lint and style): We are using black and ruff for linting. To automate the process a bit we also added a .pre-commit-config.yaml file, which can be installed in your local xradar git repo (refer to pre-commit docs). If the linter is happy unit-tests are run.

[codecov[bot]]

Codecov Report

Merging #96 (f28d567) into main (36071a8) will increase coverage by 0.29%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
+ Coverage   88.17%   88.46%   +0.29%     
==========================================
  Files          19       19              
  Lines        3289     3294       +5     
==========================================
+ Hits         2900     2914      +14     
+ Misses        389      380       -9     

Flag	Coverage Δ
unittests	88.46% <100.00%> (+0.29%)	:arrow_up:

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

Impacted Files	Coverage Δ
xradar/io/export/odim.py	96.69% <100.00%> (+0.14%)	:arrow_up:
xradar/io/backends/rainbow.py	92.30% <0.00%> (+2.16%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[egouden]

Thank you @kmuehlbauer for the quick review.

I added an explicit check for mandatory radar identifiers. I would not add more complex tests at this stage.

I am not testing reading back the written file because I see no direct equivalent in the model based on FM301.

I would like to add a test for the exception. Do you have any recommendations?

[kmuehlbauer]

Thanks @egouden!

I added an explicit check for mandatory radar identifiers. I would not add more complex tests at this stage.

Yes, we can iterate on that when needed later.

I am not testing reading back the written file because I see no direct equivalent in the model based on FM301.

You're right, there is no direct equivalent in the current standard. That's something we might solve by adding to xarray's encoding attribute. This needs some thought, though. Would you mind opening an issue for that, so we do not forget about it?

The PullRequest is looking good to me. Please add an entry in https://github.com/openradar/xradar/blob/main/docs/history.md. I will let this sit another day or two, so that others can have a look too, before merge.

[kmuehlbauer]

I would like to add a test for the exception. Do you have any recommendations?

You can try to call to_odim with broken source-kwargs.

Then you can use pytest-raises to see if the Exception was raised correctly.

[kmuehlbauer]

@egouden Let me know if you are fine with it now. I'll have @mgrover1 have another look before merge.

[kmuehlbauer]

Thanks @egouden, welcome on board xradar.