Add support for land cover data #73
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| return { | ||
| "LCCS_landcover": np.array( | ||
| [landcover_lookup_table[_id] for _id in lccs_id.to_numpy()] | ||
| ), | ||
| "IGBP_veg_long": np.array( | ||
| [igbp_lookup_table[_id] for _id in lccs_id.to_numpy()] | ||
| ), | ||
| } | ||
|
|
There was a problem hiding this comment.
Since LCCS_landcover is unused in the code, can this function only returns IGBP_veg_long as a np.array()?
There was a problem hiding this comment.
Well, I thought it would be good to keep it in for now, also based on the discussion in EcoExtreML/STEMMUS_SCOPE#138.
It's easier than having to dive into the code again later, to then add it in.
It would also be nice (in general) if these land cover data could be in the output netCDF, for provenance sake.
There was a problem hiding this comment.
Yes, when the converter is implemented, it can be added as attrs there. Also, it should be added to the doc as mentioned in #78.
| timestep, | ||
| ) | ||
| data["IGBP_veg_long"] = landcover_data["IGBP_veg_long"][0] | ||
| data["LCCS_landcover"] = landcover_data["LCCS_landcover"][0] |
There was a problem hiding this comment.
| data["LCCS_landcover"] = landcover_data["LCCS_landcover"][0] |
There was a problem hiding this comment.
Because the code doesn't use it.
| time_range, | ||
| timestep, | ||
| ) | ||
| data["IGBP_veg_long"] = landcover_data["IGBP_veg_long"][0] |
There was a problem hiding this comment.
Let's get landcover_data["IGBP_veg_long"] as a np.array instead of a dictionary. See my comment above.
| @@ -0,0 +1,24 @@ | |||
| "lccs_class","IGBP_STEMMUS_SCOPE" | |||
There was a problem hiding this comment.
We should explain how this file is generated. I suggest adding a section "Dataset" to the documentation and explaining all datasets there. This can be done in another item. see issue #78
There was a problem hiding this comment.
@BSchilperoort thanks. it looks very good. The run was successful. Just some minor comments
Co-authored-by: SarahAlidoost <[email protected]>
|
Kudos, SonarCloud Quality Gate passed! |
This PR adds support for the CCI land cover dataset.
The following have been added:
I decided to remove the dem and canopy height text files, as those have been integrated into the package (as compressed txt files).