fix issue #69 Warn.zeropadding for islands #70
Conversation
There was a problem hiding this comment.
Thanks for extending the warn.zeropadding functionality to island!
- I found a Ctrl+C error in your check whether latitudes are represented accurately enough.
- The rest are minor comments. Personally I like to have a comment above every block of code to ensure easy understanding of the code. But this might not correspond to the conventions?
- You have added test for the two new functionalities: (1) zeropadding for islands (i.e., patchy fields in general); (2) check for spatially sufficiently resolved input values. Thus, testing looks fine to me.
After fixing the Ctrl+C error and having considered whether to adopt the minor comments or not, we can merge the branch.
climada_petals/engine/warn.py
Outdated
|
|
||
| i = ((lat - min(lat)) / abs(grid_res_lat)).astype(int) | ||
| j = ((lon - min(lon)) / abs(grid_res_lon)).astype(int) | ||
| map_rec = np.zeros((len(un_y), len(un_x))) | ||
| map_rec[i, j] = val | ||
|
|
||
| xx, yy = np.meshgrid(un_x, un_y) |
There was a problem hiding this comment.
Linter suggests to rename xx and yy. Maybe he approves more of un_x_flat, un_y_flat?
There was a problem hiding this comment.
I removed these two intermediate variables. Now we got rid of two Linter issues.
Co-authored-by: leonie-villiger <87335708+leonie-villiger@users.noreply.github.com>
check_lat was calculated using the lon variables due to a copying error. This was corrected and it is now calculated correctly using the lat variables Co-authored-by: leonie-villiger <87335708+leonie-villiger@users.noreply.github.com>
|
Thanks Leonie, for the very well done review. Do you have time to check my changes? |
|
Your changes resolve the spotted issues and the code is clear and comprehensible. In this regard, all good to merge! However, when checking the |
|
Good catch, I added some asserts for coord as well. I am now merging without further review. Thanks alot for your suggestions and for your approval. |
This pull requests fixes #69
the function
Warn.zeropaddingwas adopted to now deal with cases where the inputed non-rectangular map contains several non touching areas like islands. This produced an IndexError before. A test is added to check that such islands are handled correctly.As it was already the case beforehand, the function assumes that the inputed non-rectangular map contains lat and lon values, that can be accurately represented as a regular grid. It now also checks for this assumption with a maximally allowed relative error in terms of grid resolution. This check is also tested.