Implementing a more flexibel data ingest for a_b() #141
Conversation
|
Thanks to the tests i found a strange behavior of |
|
Good to know! But it is actually not a "strange behavior" because the docs of
So you have to specify |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #141 +/- ##
==========================================
+ Coverage 74.53% 75.17% +0.64%
==========================================
Files 29 29
Lines 1084 1112 +28
==========================================
+ Hits 808 836 +28
Misses 276 276
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
Further update of the function: when |
There was a problem hiding this comment.
Thanks @maxmargraf. Looks good. I have some small comments regarding required adjustments.
pycomlink/processing/k_R_relation.py
Outdated
| b[pol_mask_v] = b_v[pol_mask_v] | ||
|
|
||
| if return_xarray: | ||
| if tmp_dims_f_GHz != (): |
There was a problem hiding this comment.
Please add a comment with an explanation here, why this is needed.
| else: | ||
| return_xarray = False | ||
|
|
||
| f_GHz = xr.DataArray(f_GHz) |
There was a problem hiding this comment.
We allow certain combinations of np.ndarray, xr.DataArray and float or str. But other combinations are not allowed. I suggest to add that info to the docstring at f_GHz and pol.
I know that we do not mention xr.DataArray as input data type in many docstrings, even though they are handled via the xarray-wrapper decorator. I actually always wanted to add the featuer that this wrappers add a note about that behavior to the docstring of the wrapped function. Yet another todo... But here, since we actually handled xr.DataArrays expliclity in the function body, I would adjust the docstring accordingly.
pycomlink/processing/k_R_relation.py
Outdated
|
|
||
| f_GHz = np.asarray(f_GHz) | ||
| if tmp_dims_f_GHz != tmp_dims_pol: | ||
| raise ValueError("Frequency and polarization must have identical " |
There was a problem hiding this comment.
This is not yet covered by a test. Please add one simple test for that.
There was a problem hiding this comment.
Should be fixed already by commit 7440056. This code coverage report is three hours old right now.
|
thanks for this enhancement 👍 |
) * remove test of removed function * update nearby wet dry test and remove test for dims for input as only one variable is passed anyway now * require pmax in power correction * update test to proper pmax behavior * update nearby example notebook * update rainfall retrival test with more realistic pmax values * adjust code to use new a_b() function from #141 * add xarray wrapper decorator to handle sublink dimension
This PR is solving #140.
a_b()previously was not flexible in regard of data type of input. Nowxr.DataArray,np.array,float(for frequency) andstr(for polarization) are allowed. Tests are updated to also checkxr.DataArrayas input (also withstrvor polarization) and one new raise.This also will help to slacken
nearby_rain_retrival().ToDo: