Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: update join_radar to also append instrument parameters #1204

Merged
merged 7 commits into from
Jul 19, 2022
Merged

FIX: update join_radar to also append instrument parameters #1204

merged 7 commits into from
Jul 19, 2022

Conversation

wolfidan
Copy link
Contributor

@wolfidan wolfidan commented Jul 15, 2022

Hi dear Py-ART team,

The join_radar function in Py-ART has the problem that it does not merge instrument parameters. This is an issue for certain functions for example dealias_region_based, which expects a nyquist_velocity value for every sweep. I think there is currently no way to use this function on a multi sweep scan obtained with join_radar. In our MeteoSwiss version we use the function in this PR, which also merges the most important instrument parameters and is able to merge radar objects which have different variables (by merging only the variables that they have in common).
The code below for example fails for me with the current Py-ART version with error

nyquist_vel = [radar.get_nyquist_vel(i, check_uniform) for i in
return(nyq_vel[0])
TypeError: only size-1 arrays can be converted to Python scalars

but works with this fix.


for i,f in enumerate(files):
    radar =  pyart.io.read_cfradial(f)
    if i == 0:
        mradar = radar
    else:
        mradar = pyart.util.join_radar(mradar, radar)
    
    
wind = pyart.correct.dealias_region_based(mradar, ref_vel_field = 'velocity')

Cheers,
Daniel

@wolfidan wolfidan changed the title update join_radar to also append instrument parameters FIX: update join_radar to also append instrument parameters Jul 15, 2022
@wolfidan
Copy link
Contributor Author

Sorry warn function is missing, I'll fix that ASAP.

@zssherman
Copy link
Collaborator

Thanks @wolfidan for doing this! Yeah once the warn is fix illl do another review.

@zssherman
Copy link
Collaborator

zssherman commented Jul 15, 2022

@wolfidan Looks good, only thing is there is an added whitespace near the top making 3 spaces between the two functions. More just nitpicking, but besides that everything looks good.

@zssherman zssherman requested review from zssherman July 15, 2022 16:51
Copy link
Collaborator

@mgrover1 mgrover1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wolfidan thanks for adding this fix! One small suggestion - could you please add a test for this?

@zssherman zssherman closed this Jul 18, 2022
@zssherman zssherman reopened this Jul 18, 2022
@zssherman
Copy link
Collaborator

Doing a test on CI, why there was a close and reopen

@wolfidan
Copy link
Contributor Author

wolfidan commented Jul 19, 2022

Thank you @mgrover1, I have added an unittest that check if a join of two radars with different numbers of sweeps and only one common field is possible. 88ff2bd

Copy link
Collaborator

@mgrover1 mgrover1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks @wolfidan

@zssherman zssherman merged commit 4bfa4dc into ARM-DOE:main Jul 19, 2022
mgrover1 added a commit that referenced this pull request Aug 24, 2022
* add cookbook section (#1199)

* TST: Adding unitests for rain rate module. (#1200)

* fix missing < symbol in link (#1201)

* add pypi stats to readme (#1206)

* FIX: update join_radar to also append instrument parameters (#1204)

* updated join_radar to also append instrument parameters

* updated join_radar to also append instrument parameters

* add pypi stats to readme (#1206)

* updated join_radar to also append instrument parameters

* updated join_radar to also append instrument parameters

* added unittest for join_radar

Co-authored-by: Max Grover <[email protected]>

* TST: Moving tests to test module within Py-ART main directory. (#1207)

* TST: Moving pyart main module tests to test directory.

* TSTS: Move bridge and core submodules to respective test directories.

* TSTS: Moving correct and filters tests to respective tests directories.

* TST: Moving graph io and map test modules.

* TST: Moving retrieve, testing and util submodule tests.

* FIX: Remove pyargs

* FIX: Fix pytest call.

* FIX: Remove extra netcdf reference.

* MNT: Remove duplicate pytest as well.

* MNT: Changing custom config path.

* FIX: Fix for config loading and not resetting.

* TST: Adding back in test_simualted_vel.

* FIX: Fix for missing sys paths.

* FIX: Attemped PIL fix.

* MNT: Remove pillow.

* FIX: Fix for only python 3.10 being used. (#1208)

* FIX: Fix for only python 3.10 being used.

* FIX: Adding python call.

* MNT: Another attempt.

* FIX: Xarray pin removed.

* MNT: Remove cftime pin.

* MNT: Attemped fix for pil.

* MNT: Accidently removed gdal

* MNT: Pinning for now.

* MNT: Remove lib set.

* CI: Remove python 3.6 from testing.

* remove pinning for specific cftime (#1211)

* DOC: Updating Py-ART's how to release doc and removing most of ci_setup doc (#1213)

* DOC: Updating Py-ART's how to release doc and removing most of ci_setup.

* DOC: Changes reflecting review.

* ENH: Corrected Height Gates to be height above sea level (#1215)

* ENH: Corrected Height Gates to be height above sea level

* FIX: Removed character within comment line

* ENH: Updated Height Test to include altitude of radar.

* TST: Fix median filter depreciation warnings as well as more tests for simple moments. (#1214)

* TST: Fix median filter depreciation warnings as well as more tests for
simple moments.

* TST: Update values.

* MNT: Adding variables for simple moment calculation functions.

* FIX: Removal of depreciation warning for interp namespace.

* fix writing IRIS sigmet files to UF (#1216)

* FIX: join_radar also joins pulse repititon times iparam (#1218)

* Add pyrad to the readme (#1220)

* add pyrad to the readme

* move pyrad to extension section

* add space between _ and :

* FIX: Return zero phase for sweeps completely filtered out with gatefi… (#1226)

* FIX: Return zero phase for sweeps completely filtered out with gatefilter.

* FIX: Remove debug statement

* Trmm test (#1228)

* TST: To see if trmm_rsl broke.

* MNT: Forgot final else.

* MNT: Remove full path.

* add hardcoded trmm_rsl path to docs

* add missing RSL_PATH=

* upin cftime

* build docs on macos

* update to macos hash

* add macos latest runner for docs

* add macOS-latest

* move toward micromamba

* change path to micromamba

* add ls and move to mamba exe

* try pkgs dir

Co-authored-by: mgrover1 <[email protected]>
Co-authored-by: Max Grover <[email protected]>

* remove load module section (#1234)

* DOC: Updated pyart doc to include column code, fix incorrect syntax and more. (#1231)

* DOC: Updated pyart doc to include column code, incorrect syntax and
more.

* DOC: Forgot https

* MNT: Revert ssh to https

* MNT: Suggested changes.

* ADD: addition of subset_radar function in radar_utils (#1227)

* added cut_radar function in util

* added cut_radar function in util

* renamed cut_radar to subset_radar

* MNT: Update ci.yml.

* MNT: Updating environments.

* MNT: Duplicate 'latest'

* MNT: Revert back os runner check.

* MNT: Add bash to each run

Co-authored-by: zssherman <[email protected]>

* MNT: Remove trmm_rsl from CI optional, remove from debug info. (#1239)

* CI: Remove trmm_rsl from testing.

* MNT: Remove trmm_rsl from debug and install warning build message.

* CI: Remove trmm_rsl pathing from CI.

* MNT: Remove warning from setup.py

* remove trmm_rsl from doc env

* MNT: Don't code cover deprecated code.

* replace dealias with region based

* use cfradial with rhi two panel

* add new plot ppi cfradial remove old one using sigmet

* use cfradial with xsect example

Co-authored-by: mgrover1 <[email protected]>

* add tests for xradar

* make sure CI runs on the xradar branch

* add datatree as pip and fix missing import

* remove python 3.7

* fix incorrect xradar test

* update __init__.py

* remove chunk arg

Co-authored-by: Zach Sherman <[email protected]>
Co-authored-by: Daniel Wolfensberger <[email protected]>
Co-authored-by: Joe O'Brien <[email protected]>
Co-authored-by: Sam Gardner <[email protected]>
Co-authored-by: Bobby Jackson <[email protected]>
Co-authored-by: zssherman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants