openradar / xradar

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

ENH: Add a reader for nexrad level2 files #147

Closed mgrover1 closed 3 months ago

mgrover1 commented 6 months ago

This is a first cut at the nexrad file reader... starting with level2 data. Still a work in progress for now, but I figured I would share what I have so far. I may have more questions about backends @kmuehlbauer and how to deal with loading in the py-art like dictionaries.

mgrover1 commented 5 months ago

@kmuehlbauer - I am still struggling to get this up and running... do you think there would be some utility to have a generic RadarDataStore object, similar to the pyart.base.Radar object that could port dictionaries --> xarray backends?

For example, taking the following as input:

RadarDataStore(
        time,
        _range,
        fields,
        metadata,
        scan_type,
        latitude,
        longitude,
        altitude,
        sweep_number,
        sweep_mode,
        fixed_angle,
        sweep_start_ray_index,
        sweep_end_ray_index,
        azimuth,
        elevation,
        instrument_parameters=instrument_parameters,
    )

It would return an xarray data structure? I am having trouble decoupling the entry point commonalities from the individual file parsing structures in the current backends...

Some benefits of moving toward this approach would be:

kmuehlbauer commented 5 months ago

@mgrover1 I've had not yet time to check this PR out. Hopefully I can free up some time next week.

If such object makes code readability and maintenance easier, why not. How would that be integrated into xarray-backend machinery?

mgrover1 commented 5 months ago

My thought right now is that it would be structured as:

NexradLevel2File --> RadarDataStore --> NexradBackendEntrypoint

The benefit here would be that the RadarDataStore would be the primary object that we would fit the coordinates + fields into... then we can pass that into the backend entrypoint. I can prototype this in this PR, and ping you when it is ready for feedback?

mgrover1 commented 5 months ago

@kmuehlbauer - I ended up going with a function instead of a class... it takes in the same things the radar object did in Py-ART, and returns an xarray.Dataset, with a group argument that can be used to specify which sweep to use... this way, it can be used directly with the backend entrypoint API... the user can then add additional bits to their dataset before returning that to the user. Open to thoughts here!

mgrover1 commented 5 months ago

@kmuehlbauer - I am stumped on what I am doing wrong here for it not to recognize the cython submodule I added.

codecov[bot] commented 5 months ago

Codecov Report

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

Project coverage is 86.28%. Comparing base (a854715) to head (5c7bd7e).

:exclamation: Current head 5c7bd7e differs from pull request most recent head 2a1c46e. Consider uploading reports for the commit 2a1c46e to get more accurate results

Files Patch % Lines
xradar/io/backends/nexrad_level2.py 75.76% 119 Missing :warning:
xradar/io/backends/common.py 57.62% 25 Missing :warning:
xradar/io/backends/nexrad_common.py 40.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #147 +/- ## ========================================== - Coverage 90.79% 86.28% -4.51% ========================================== Files 20 22 +2 Lines 3421 3995 +574 ========================================== + Hits 3106 3447 +341 - Misses 315 548 +233 ``` | [Flag](https://app.codecov.io/gh/openradar/xradar/pull/147/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/147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openradar) | `?` | | | [unittests](https://app.codecov.io/gh/openradar/xradar/pull/147/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openradar) | `86.28% <73.60%> (-3.64%)` | :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.

mgrover1 commented 5 months ago

@mgrover1 We knew that it would not be an easy task. I've added another couple of suggestions and ideas.

It looks like we need to tackle the different gate spacing stuff at a lower level.

Thanks for the suggestions - I am busy with the AMS conference today, but will follow up later this week. I appreciate the feedback! Agreed - it is not easy, but will be worth it :)

kmuehlbauer commented 5 months ago

Thanks for the suggestions - I am busy with the AMS conference today, but will follow up later this week. I appreciate the feedback! Agreed - it is not easy, but will be worth it :)

No worries, Max. I'm still getting accustomed to the code and we'll surely have further iteration cycles here. And you are absolutely right, it will be worth it.

mgrover1 commented 3 months ago

Closing this since @kmuehlbauer refactored + submitted with #158