Feature/add spatial disaggregation recipe #57
Conversation
|
@dgergel - I just merged some changes in that should fix the CI for you. Go ahead and pull those in and then I'll take a look here. |
…into feature/add_spatial_disaggregation_recipe
|
@jhamman sorry for the delay in fixing the |
There was a problem hiding this comment.
This looks cool. Just a few comments to tidy things a bit but let's merge this soon. I suspect some iteration around the spatial models api will be needed but its good to start somewhere.
Oh! And tests, can you pull a basic set of unit tests together for the SpatialDisaggregator?
skdownscale/spatial_models/sd.py
Outdated
| if var == 'temperature': | ||
| pass | ||
| elif var == 'precipitation': | ||
| pass | ||
| else: |
There was a problem hiding this comment.
Let's write this as:
| if var == 'temperature': | |
| pass | |
| elif var == 'precipitation': | |
| pass | |
| else: | |
| if var not in ['temperature', 'precipitation'] : |
skdownscale/spatial_models/sd.py
Outdated
| if not np.array_equal(ds_bc[lat_name], climo_coarse[lat_name]): | ||
| raise ValueError('climo latitude dimension does not match model res') | ||
| if not np.array_equal(ds_bc[lon_name], climo_coarse[lon_name]): | ||
| raise ValueError('climo longitude dimension does not match model res') |
There was a problem hiding this comment.
maybe something like this:
| if not np.array_equal(ds_bc[lat_name], climo_coarse[lat_name]): | |
| raise ValueError('climo latitude dimension does not match model res') | |
| if not np.array_equal(ds_bc[lon_name], climo_coarse[lon_name]): | |
| raise ValueError('climo longitude dimension does not match model res') | |
| if not ds_bc[lat_name].equals(climo_coarse[lat_name]): | |
| raise ValueError('climo latitude coordinate does not match model res') | |
| if not ds_bc[lon_name].equals(climo_coarse[lon_name]): | |
| raise ValueError('climo longitude coordinate does not match model res') |
| specifies the variable being downscaled. Default is | ||
| temperature and other option is precipitation. |
There was a problem hiding this comment.
minor nit but these are over-indented (same for all docstrings here)
There was a problem hiding this comment.
I think this is fixed now.
…re/add_spatial_disaggregation_recipe
Codecov Report
@@ Coverage Diff @@
## main #57 +/- ##
==========================
==========================
Continue to review full report at Codecov.
|
|
@jhamman so sorry for taking so long to get back to this. I've made the changes you suggested and added unit tests, so this is ready for another review. |
This PR adds the following:
SpatialDisaggregatorclass in spatial models