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

DataTree to Dataset transformation method #133

Open syedhamidali opened 9 months ago

syedhamidali commented 9 months ago

Proposal to Implement Data Transformation Method from cfradial2 to cfradial1 and vice versa

Dear Xradar Team,

I would like to propose the introduction of a data transformation method that facilitates smooth conversion from the current cfradial2 format to the legacy cfradial1 format within the Xradar library. This addition would greatly enhance user experience and improve compatibility with existing code. Specifically, it would enable users to perform operations on the entire volume, i.e., across all radar sweeps. However, it's essential to ensure that these operations are reversible, allowing for seamless transitions between datatree to dataset and dataset to datatree formats.

The proposed method would be accessible through either of the following options:

To address this, I suggest creating a new module named cfradial1_model.py within the root directory. This module would incorporate the code currently residing in xradar/io/export/cfradial1.py and provide a method to return the dataset exclusively. This approach would ensure a clean separation of concerns.

Additionally, for the export to_cfradial1 functionality, we could import the necessary function from cfradial1_model.py and extend it with the option to save the dataset to a NetCDF file, like so: ds.to_netcdf(file). I can easily take care of datatree to dataset transformation, but would need help with the datatree to dataset transformation.

By adopting this proposal, I aim to simplify the process of transitioning between cfradial2 to cfradial1 formats, and vice versa, within the Xradar library, without affecting the current method to export the cfradial1 data.

Your feedback and suggestions are highly appreciated, and looking forward to your thoughts on this proposal.

Best, Hamid

mgrover1 commented 9 months ago

I agree this functionality would be helpful!

kmuehlbauer commented 9 months ago

I totally support this. I have one suggestion so far. The function naming is too general IMHO. What about .to_cf1_dataset() and to_cf2_datatree() or even shorter .to_cf1(), to_cf2()?

We would need to be careful in the creation process wrt data copies/views. That might need some thinking on the use cases.

mgrover1 commented 9 months ago

I would argue for .to_cfradial1() or .to_cfradial2() to be more clear...

kmuehlbauer commented 9 months ago

I can easily take care of datatree to dataset transformation, but would need help with the datatree to dataset transformation.

I've stumbled over this on the second read. Where do you exactly need help, datatree -> dataset or dataset -> datatree?

kmuehlbauer commented 9 months ago

I would argue for .to_cfradial1() or .to_cfradial2() to be more clear...

I see the point. Then .to_cfradial1_dataset() / to_cfradial2_datatree() is more explicit and can't be confused with to_cfradial1/to_cfradial2 export functions.

Concerning custom methods, those would have to be implemented as accessors.

mgrover1 commented 9 months ago

I would argue for .to_cfradial1() or .to_cfradial2() to be more clear...

I see the point. Then .to_cfradial1_dataset() / to_cfradial2_datatree() is more explicit and can't be confused with to_cfradial1/to_cfradial2 export functions.

Concerning custom methods, those would have to be implemented as accessors.

I like that idea!! Agreed!

syedhamidali commented 9 months ago

I can easily take care of datatree to dataset transformation but would need help with the datatree to dataset transformation.

I've stumbled over this on the second read. Where do you need help, datatree -> dataset or dataset -> datatree?

I can easily implement the conversion from a datatree -> dataset, as I've already done that in the export function. However, using it might require calling something like xd.to_dataset(dtree). What I need assistance with is making it more user-friendly so we can use it more intuitively, such as dtree.to_cfradial1_dataset(); as you already mentioned it could be achieved through accessors.

As for converting a dataset -> datatree, I'm not sure how to do it right now, but I'm willing to give it a try. Let's create a basic framework for this task more like a skeleton. Maybe you, @kmuehlbauer, or @mgrover1 could lead this effort. We need a straightforward structure, like a tree with branches, to simplify the implementation process.

Something like this

xradar
└── xradar
    ├── __init__.py
    ├── accessors.py
    ├── georeference
    ├── io
    ├── transform or models or data_models 
    │   └── cfrad2_to_cfrad1.py
    └── xradar.py

cfrad2_to_cfrad1.py

Class Cfradial1Model
def __init__
def cfradial1
syedhamidali commented 8 months ago

I tried experimenting with it, and it appears to be functioning as anticipated. Currently, it resides within the 'transform' branch of my xradar fork. If you're interested, you can also check out this notebook to see how it operates and what results we're getting.

import xradar as xd
from open_radar_data import DATASETS
f = DATASETS.fetch("cfrad.20080604_002217_000_SPOL_v36_SUR.nc")
dtree = xd.io.open_cfradial1_datatree(f)
dtree
Screenshot 2023-10-10 at 11 06 23 AM
ds = dtree.to_cfradial1_dataset()
ds
Screenshot 2023-10-10 at 11 07 43 AM
dtreenew = ds.to_cfradial2_datatree()
Screenshot 2023-10-10 at 11 09 00 AM
type(dtreenew)

xradar.transform.cfradial.Cfradial2

ds = dtreenew.to_cfradial1_dataset()
type(ds)

xradar.transform.cfradial.Cfradial1

syedhamidali commented 8 months ago

@kmuehlbauer @mgrover1 What are your thoughts on it? Should I make a pull request and create a separate branch before finalizing it?

mgrover1 commented 8 months ago

@syedhamidali - this looks great! I would suggest opening a PR and then we can review!! Nice Work!!