openradar / xradar

A tool to work in weather radar data in xarray
https://docs.openradarscience.org/projects/xradar
MIT License
97 stars 17 forks source link

NEXRAD Level2 structured reader #158

Closed kmuehlbauer closed 6 months ago

kmuehlbauer commented 6 months ago

This is a rewrite of #147 using parts of the structured reader of the iris/sigmet reader. This also enhances the ability of reading much more metadata from the nexrad level 2 archive.

Note: this only implements MSG_31 as of now. MSG_1 need to be included after adding such file to open-radar-data repo.

There is also relevant code over in https://github.com/jthielen/xradar/tree/nexrad-level2 by @jthielen, which should be added to this structured reader in a follow-up PR.

kmuehlbauer commented 6 months ago

Finally linting is good. Had outdated version on my machine :grimacing:

kmuehlbauer commented 6 months ago

@mgrover1

This only reads the absolutely necessary metadata to get a hunch on sweeps/moments. Everything else is read when first requested by xarray (so it's lazy too). The reader uses numpy.memmap (like the sigmet reader). Will push the fixture in a minute...

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 94.62963% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 91.30%. Comparing base (a854715) to head (63301e9).

Files Patch % Lines
xradar/io/backends/nexrad_level2.py 94.51% 29 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #158 +/- ## ========================================== + Coverage 90.79% 91.30% +0.51% ========================================== Files 20 21 +1 Lines 3421 3956 +535 ========================================== + Hits 3106 3612 +506 - Misses 315 344 +29 ``` | [Flag](https://app.codecov.io/gh/openradar/xradar/pull/158/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openradar) | Coverage Δ | | |---|---|---| | [notebooktests](https://app.codecov.io/gh/openradar/xradar/pull/158/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openradar) | `80.71% <87.96%> (+1.11%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/openradar/xradar/pull/158/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openradar) | `89.61% <87.77%> (-0.31%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openradar#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kmuehlbauer commented 6 months ago

@mgrover1 Somehow my rebase threw away to much. Finally this is working again. Maybe we can talk in the next days, if necessary?

mgrover1 commented 6 months ago

yes! next week's radar community meeting?

review-notebook-app[bot] commented 6 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

mgrover1 commented 6 months ago

The docs and test coverage look great - we can continue to iterate + test through other PRs - what do you think about merging @kmuehlbauer ?

kmuehlbauer commented 6 months ago

Let's wait after the meeting today. There are some issues which I want to show before we get this in.

mgrover1 commented 6 months ago

I am putting together a notebook right now comparing the data fields from Py-ART + xradar here as well... which should help us understand some of the differences?

kmuehlbauer commented 6 months ago

I am putting together a notebook right now comparing the data fields from Py-ART + xradar here as well... which should help us understand some of the differences?

Great idea! One major difference is that this PR does not mask the data. We would need to think how we want to handle masking in the future. But we can make first make a list with the current differences.

kmuehlbauer commented 6 months ago

@mgrover1 If you think we should move forward from here, please go ahead and merge. I'll add several issues dedicated to different aspects/downsides of this implementation. Please also add any issues you find to the tracker, so that we can work on fixing enhancing.

mgrover1 commented 6 months ago

Great!! Agreed 😄

mgrover1 commented 6 months ago

@kmuehlbauer - should we close out #40 here and open new issues related to more specific improvements?

kmuehlbauer commented 6 months ago

Or let's use this issue (#40) as a tracking issue. We could list all related PR/issues there? Can also pin that issue to the top. WDYT? I'm also good with closing #40.

mgrover1 commented 6 months ago

Ohh I like that idea... that can be the issue we refer back to + scaffold out. We can rename it to NEXRAD Support, and refer to the different readers and improvements.

mgrover1 commented 6 months ago

I created #160 to reflect this single reader, and modified #40 accordingly. Sound reasonable @kmuehlbauer ?

kmuehlbauer commented 6 months ago

@mgrover1 Yes, sounds good.