enh: add dimension alignment support for swap_dims_channel_frequency#1500
enh: add dimension alignment support for swap_dims_channel_frequency#1500praneethratna wants to merge 3 commits into
swap_dims_channel_frequency#1500Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1500 +/- ##
===========================================
+ Coverage 83.52% 94.92% +11.39%
===========================================
Files 64 5 -59
Lines 5686 315 -5371
===========================================
- Hits 4749 299 -4450
+ Misses 937 16 -921
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| - Winter2017-D20170115-T150122.raw: Contains a change of recording length in the middle of the file | ||
| - 2015843-D20151023-T190636.raw: Not used in tests but contains ranges are not constant across ping times | ||
| - SH1701_consecutive_files_w_range_change: Not used in tests. [Folder](https://drive.google.com/drive/u/1/folders/1PaDtL-xnG5EK3N3P1kGlXa5ub16Yic0f) on shared drive that contains sequential files with ranges that are not constant across ping times. | ||
| - NBP_B050N-D20180118-T090228.raw: split-beam setup without angle data |
There was a problem hiding this comment.
I am not sure why these keep on happening (I found these on my local machine too. I think it might be due to me not setting up the docker container again and just working with the files I already have, so the new ones did not get pulled in). Could you please put these lines back? Thanks!
| if "channel" not in source_Sv.variables: | ||
| raise ValueError("The input source_Sv Dataset must have a channel dimension!") | ||
|
|
||
| # Select ds_beam channels from source_Sv | ||
| # Swap the dimension to frequency_nominal if use_frequency_nominal is True | ||
| if use_frequency_nominal: | ||
| source_Sv = swap_dims_channel_frequency(source_Sv) |
There was a problem hiding this comment.
What I was thinking was to make the add_splitbeam_angle function agnostic of whether in the input ds_Sv dataset has a dimension of channel or frequency_nominal. So the function will work with the original dimension (channel) straight from compute_Sv, and will also work if the dimension has already been changed to frequency_nominal before the add_splitbeam_angle function is called. So not running the swap_dims_channel_frequency under the hood within this function, but made this functions accepts ds_Sv with either dimensions.
Does the above make sense to you? So basically you can remove the added use_frequency_nominal argument and just have the function accepting both channel and frequency_nominal under the hood.
leewujung
left a comment
leewujung
left a comment
There was a problem hiding this comment.
@praneethratna : I checked the changes and elaborated more on the intention of the issue. Let me know if this comment clarifies it! Thanks!
|
@leewujung I'm not sure what's left to address in this issue. It seems like my PR #1520 which resolves #1488 also applies to this issue. Let me know if I'm missing something in this issue though. |
|
@dbashford-NOAA - oh that's true, I guess that didn't register because I haven't reviewed #1520. I'll try to get to that tomorrow. Thanks! |
|
Closing this as it is superseded by #1520. |
4 participants
Addresses #1488 and
swap_dims_channel_frequencynow supports dimension alignment.