ENH: Add a reader for nexrad level2 files #147

mgrover1 · 2024-01-05T21:10:24Z

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.

Closes Add NEXRAD Support #40
Tests added
Changes are documented in history.md

mgrover1 · 2024-01-17T15:17:01Z

@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:

The user just needs to extract dictionaries with those variables (we already have many of these in the base data model)
We can refactor the existing readers to use this approach, cutting down on the duplicated Variable()... code, as well as handling the different dimensions (azimuth vs. time)
We can add a well-defined "how to add an xradar xarray backend" section to the docs that walks through how to fit this into the data model
Easy to port over more readers from Py-ART

kmuehlbauer · 2024-01-18T10:15:03Z

@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 · 2024-01-18T14:10:56Z

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 · 2024-01-18T22:01:33Z

@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 · 2024-01-25T22:03:23Z

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

codecov · 2024-01-26T01:25:03Z

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).

❗ 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 ⚠️
xradar/io/backends/common.py	57.62%	25 Missing ⚠️
xradar/io/backends/nexrad_common.py	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            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	Coverage Δ
notebooktests	`?`
unittests	`86.28% <73.60%> (-3.64%)`	⬇️

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.

kmuehlbauer

@mgrover1 Thanks for moving this forward. I've added a couple of comments.

I'm still not convinced we need the cythonized interpolation at all. It looks like it is only needed to conform the single sweeps onto a common range resolution. This won't be needed for our sweep based data model. Or am I missing something?

So in case my assumption is correct, I'd suggest to shape the code to keep the original sweep resolution and remove all Cython related things from this PR.

kmuehlbauer · 2024-01-26T07:46:43Z

xradar/io/backends/__init__.py

@@ -24,5 +25,7 @@
 from .iris import *  # noqa
 from .odim import *  # noqa
 from .rainbow import *  # noqa
-
-__all__ = [s for s in dir() if not s.startswith("_")]


Why remove the __all__ here?

Not sure... it is back in. thanks!!

kmuehlbauer · 2024-01-26T08:25:31Z

xradar/io/backends/nexrad_level2.py

+
+    # range
+    _range = get_range_attrs()
+    first_gate, gate_spacing, last_gate = _find_range_params(scan_info)


duplicate of line 1072?

Yep - should be fixed

kmuehlbauer · 2024-01-26T08:31:24Z

xradar/io/backends/nexrad_level2.py

+    # fields
+    max_ngates = len(_range["data"])
+    available_moments = {m for scan in scan_info for m in scan["moments"]}
+    interpolate = _find_scans_to_interp(


Looks like this is already done above at line 1075?

See the latest commit - that should fix the duplication

kmuehlbauer · 2024-01-26T08:35:07Z

xradar/io/backends/nexrad_level2.py

+                warnings.warn(
+                    "Gate spacing is not constant, interpolating data in "
+                    + f"scans {interp_scans} for moment {moment}.",


It seems, that we do not have to do interpolation here. AFAICT this is only needed for CfRadial1 data (like good old Py-ART data model). We should be safe to keep the sweep resolution as is.

I removed the interpolation in the latest commit @kmuehlbauer :) thanks for the suggestion here.

kmuehlbauer

@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.

kmuehlbauer · 2024-01-31T06:39:57Z

.github/workflows/ci.yml

@@ -22,9 +22,6 @@ jobs:
        run: |
          python -m pip install --upgrade pip
          pip install black black[jupyter] ruff


AFAIK we would need to enable jupyter notebook linting/formatting for ruff in pyproject.toml

kmuehlbauer · 2024-01-31T06:40:20Z

.github/workflows/upload_pypi.yml

@@ -30,4 +30,5 @@ jobs:
          TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}
        run: |
          python -m build
+          python setup.py build_ext --inplace


This wont be needed anymore.

kmuehlbauer · 2024-01-31T06:40:37Z

MANIFEST.in

@@ -8,4 +8,5 @@ recursive-include tests *
 recursive-exclude * __pycache__
 recursive-exclude * *.py[co]

+global-include *.pyx *pxd


This can be removed too.

kmuehlbauer · 2024-01-31T06:41:03Z

pyproject.toml


 [build-system]
 requires = [
    "setuptools>=45",
    "wheel",
    "setuptools_scm[toml]>=7.0",
+    "numpy"


numpy can be removed?

kmuehlbauer · 2024-01-31T06:49:23Z

xradar/io/backends/nexrad_level2.py

+    return _unpack_structure(buf[pos : pos + size], structure)
+
+
+def _unpack_structure(string, structure):


FYI, @mgrover, there is already similar decoding implemented over in the iris/sigmet backend. I'd suggest to align this after this PR is merged. I'd volunteer to take this on.

kmuehlbauer · 2024-01-31T07:15:00Z

xradar/io/backends/nexrad_level2.py

+                scale = np.float32(msg[moment]["scale"])
+                mask = data <= 1
+                scaled_data = (data - offset) / scale
+                return np.ma.array(scaled_data, mask=mask)


We might also get rid of the mask and masked array here, if we correctly specify missing values and/or _FillValues as attributes. Can be done as follow-up PR.

kmuehlbauer · 2024-01-31T07:25:38Z

xradar/io/backends/nexrad_level2.py

+    storage_options={"anon": True},
+    first_dimension=None,
+    group=None,
+    **kwargs,
+):
+    # Load the data file in using NEXRADLevel2File Class
+    nfile = NEXRADLevel2File(
+        prepare_for_read(filename, storage_options=storage_options)
+    )


Can we please make this depending on storage_options.

Suggested change

storage_options={"anon": True},

first_dimension=None,

group=None,

**kwargs,

):

# Load the data file in using NEXRADLevel2File Class

nfile = NEXRADLevel2File(

prepare_for_read(filename, storage_options=storage_options)

)

storage_options=None,

first_dimension=None,

group=None,

**kwargs,

):

# Load the data file in using NEXRADLevel2File Class

if storage_options is not None:

filename = prepare_for_read(filename, storage_options=storage_options)

nfile = NEXRADLevel2File(

filename

)

Maybe this is a bit more involved, as storage_options would have to be traversed to the backend reader.

kmuehlbauer · 2024-01-31T07:32:09Z

xradar/io/backends/nexrad_level2.py

+    # range
+    _range = get_range_attrs()
+    first_gate, gate_spacing, last_gate = _find_range_params(scan_info)
+    _range["data"] = np.arange(first_gate, last_gate, gate_spacing, "float32")


Oh my, this is really hard to move from the CfRadial1 to CfRadial2 data model. AFAICS _range-dict is used for all sweeps (assuming range interpolated to a common grid). This would need to be done on a per sweep basis.

This would need another round of refactoring here.

kmuehlbauer · 2024-01-31T07:36:18Z

xradar/io/backends/nexrad_level2.py

+        dic["_FillValue"] = -9999
+        if delay_field_loading:
+            dic = LazyLoadDict(dic)
+            data_call = _NEXRADLevel2StagedField(nfile, moment, max_ngates, scans)


Here is the pain point, this will extract the moments of different scans (which might be on different range resolutions.

kmuehlbauer · 2024-01-31T07:38:03Z

xradar/io/backends/nexrad_level2.py

+    )
+
+
+def create_dataset_from_fields(


This whole function assumes that all data of all sweeps is in a common range grid. This would need refactor to work on a per sweep basis.

Co-authored-by: Kai Mühlbauer <[email protected]>

mgrover1 · 2024-01-31T12:37:56Z

@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 · 2024-01-31T12:53:42Z

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.

…into add-nexrad-reader

mgrover1 · 2024-03-28T15:55:10Z

Closing this since @kmuehlbauer refactored + submitted with #158

ENH: Add first cut at nexrad reader

c9d88ec

mgrover1 marked this pull request as draft January 8, 2024 14:46

mgrover1 added 2 commits January 8, 2024 09:49

FIX: Fix incorrect module registration

f7e42d2

FIX: Fix the requirements for the package

34b52b0

ADD: Add new xarray dataset builder

9330b17

mgrover1 added 21 commits January 19, 2024 13:57

FIX: Fix last few steps

fbd942c

ADD: Add testing suite

3f8f8cf

FIX: Fix the manifest

e042f1f

FIX: Fix local import nexrad_interpolate

0c2d0dc

FIX: Fix import of interpolation

4f1d06c

ADD: Add missing cython depend

1af4649

ADD: Add updated manifest

2e37130

ADD: Ensure cython is packaged

7ae79bd

ADD: Add specific submodules

ec2261c

force reinstall

568e65b

make sure more files are included

0a56163

FIX: Fix build of cython extensions

a904c03

FIX: Fix lowercase letter

14aa01c

revert couple of settings

888fb1b

fix installation line

44647ac

ADD: Add proper import

2b8d9b9

move interpolation to its own submodule

0550e2c

ADD: Update setup

78fada3

Move to hidden module

cb94350

FIX: Fix all imports in backends

c3824d6

fix manifest

5e0da73

ADD: Add clear submodule

f1af9cd

mgrover1 added 7 commits January 25, 2024 16:07

ADD: Add imports

6c7b824

remove name in setup

f162928

remove check on version

9592148

make sure submodule is blank

045ba99

ADD: Add extra line

2f26565

be more explicit

d7d65b2

add setup.py run

3b49bc3

ADD: Add proper installation to other parts

9c11f0b

mgrover1 marked this pull request as ready for review January 26, 2024 02:15

mgrover1 added 2 commits January 25, 2024 20:37

ADD: add test for lazy dict

e1a59df

DEL: Remove unused sections of common

98090ad

kmuehlbauer reviewed Jan 26, 2024

View reviewed changes

mgrover1 added 6 commits January 30, 2024 14:46

DEL: Remove the interpolation step

528a16d

FIX: Fix linting

1b0d97b

Only use ruff for linting

bae3d19

DOC: Add addition to history file

aee82ed

ADD: add original ci back in

fc03120

DEL: Delete extra init

c33d521

kmuehlbauer requested changes Jan 31, 2024

View reviewed changes

Update xradar/io/backends/common.py

5c7bd7e

Co-authored-by: Kai Mühlbauer <[email protected]>

mgrover1 added 2 commits February 28, 2024 08:36

Merge latest updates on main

091a7c0

Merge branch 'add-nexrad-reader' of https://github.com/mgrover1/xradar …

2a1c46e

…into add-nexrad-reader

kmuehlbauer mentioned this pull request Mar 6, 2024

NEXRAD Level2 structured reader #158

Merged

3 tasks

mgrover1 closed this Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add a reader for nexrad level2 files #147

ENH: Add a reader for nexrad level2 files #147

mgrover1 commented Jan 5, 2024 •

edited

Loading

mgrover1 commented Jan 17, 2024 •

edited

Loading

kmuehlbauer commented Jan 18, 2024

mgrover1 commented Jan 18, 2024

mgrover1 commented Jan 18, 2024

mgrover1 commented Jan 25, 2024

codecov bot commented Jan 26, 2024 •

edited

Loading

kmuehlbauer left a comment

kmuehlbauer Jan 26, 2024

mgrover1 Jan 30, 2024

kmuehlbauer Jan 26, 2024

mgrover1 Jan 30, 2024

kmuehlbauer Jan 26, 2024

mgrover1 Jan 30, 2024

kmuehlbauer Jan 26, 2024

mgrover1 Jan 30, 2024

kmuehlbauer left a comment

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

kmuehlbauer Jan 31, 2024

mgrover1 commented Jan 31, 2024

kmuehlbauer commented Jan 31, 2024

mgrover1 commented Mar 28, 2024

		return _unpack_structure(buf[pos : pos + size], structure)


		def _unpack_structure(string, structure):

ENH: Add a reader for nexrad level2 files #147

ENH: Add a reader for nexrad level2 files #147

Conversation

mgrover1 commented Jan 5, 2024 • edited Loading

mgrover1 commented Jan 17, 2024 • edited Loading

kmuehlbauer commented Jan 18, 2024

mgrover1 commented Jan 18, 2024

mgrover1 commented Jan 18, 2024

mgrover1 commented Jan 25, 2024

codecov bot commented Jan 26, 2024 • edited Loading

Codecov Report

kmuehlbauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmuehlbauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgrover1 commented Jan 31, 2024

kmuehlbauer commented Jan 31, 2024

mgrover1 commented Mar 28, 2024

mgrover1 commented Jan 5, 2024 •

edited

Loading

mgrover1 commented Jan 17, 2024 •

edited

Loading

codecov bot commented Jan 26, 2024 •

edited

Loading