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 problems related to missing pol
argument when using calc_R_from_A, also did some minor cleanup of example notebook
#116
Fix problems related to missing pol
argument when using calc_R_from_A, also did some minor cleanup of example notebook
#116
Conversation
…e `pol` where it should be
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #116 +/- ##
==========================================
+ Coverage 71.62% 71.71% +0.08%
==========================================
Files 28 28
Lines 1008 1011 +3
==========================================
+ Hits 722 725 +3
Misses 286 286
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me. I only marked some typos and a small possible extension in the docstrings.
Co-authored-by: Nico Blettner <72550190+nblettner@users.noreply.github.com>
@nblettner thanks for the feedback. I adjusted the code accordingly. Sorry, that I forgot to push my last update of the notebook. It is now here in 21ed485. I just removed cells were all code was commented out. I also change the relevant parts of the code to use the solution with |
pol
argument in example processing and do some other minor improvementspol
argument when using calc_R_from_A, also did some minor cleanup of example notebook
This is required since pycomlink#116 I actually thought I had fixed that during some of the last PRs, but apparently the problem was still there in this notebook.
Things that were done in this PR:
pol
tocalc_R_from_A
when passingf_GHz
, which is the way we commonly use this function. One can also passa
andb
parameters instead off_GHz
andpol
. There are now also tests that check this behavior.waa_leijnse_2008_from_A_obs
andwaa_pastorek_2021_from_A_obs
which usecalc_R_from_A
internally, have been adjusted to require apol
argument.pol
correctly (which is no enforce anyway, see above)Datasets
to one largeDataset
because resampling is much faster like thiscml['A'].values[cml.A < 0] = 0
instead ofcml['A'] = cml.A.where(cml.A > 0, 0)
, see Do not use cml.A.where(cml.A > 0, 0) to set values smaller zero to zero #117trsl
, up to 5 minutes, which improves comparison with reference for high rain rates (I think we had this interpolation in the example notebook some time ago, but maybe it was removed to replace it with the more sophisticated blackout gap filling, which is not there yet..)