Expose marine regions version - #83
Conversation
|
Thanks for the submission @brynpickering! |
irm-codebase
left a comment
irm-codebase
left a comment
There was a problem hiding this comment.
Looks good overall!
Only thing I suggest is moving from a 'hard cap' (version==2.0.0) to a more permissive 'lower cap' (version >= 2.x.x). This allows us and users to try updating the EEZ version without needing to update the module version.
| logs="resources/module/logs", | ||
| resources="resources/module/resources", | ||
| resources=temp("resources/module/resources"), | ||
| results="resources/module/results", |
There was a problem hiding this comment.
I did not know you could pass temp() outside the call!
Does it work?
| lambda mrgid: "_".join([country_id, "marineregions", str(mrgid)]) | ||
| ), | ||
| "country_id": country_id, | ||
| "shape_class": "maritime", |
There was a problem hiding this comment.
For future proofing, we should follow this approach:
- known supported versions just call the expected process.
- versions lower than the lowest major release supported raise the RuntimeError
- versions higher throw a warning via
warnings.warn(which will go to the log file) and call the latest version (so just 2.0.0).
| marine_regions_release: | ||
| description: | | ||
| MarineRegions WFS version to use to access EEZ data. | ||
| type: string | ||
| default: "2.0.0" | ||
| enum: ["2.0.0"] |
There was a problem hiding this comment.
If my suggestions are implemented, we should replace 'enum' with a regex check based on this suggestion: semver/semver.org#431 (comment)
There was a problem hiding this comment.
Basically, we handle semver checks in code and no not 'hard cap' them, so if EEZ 2.0.1 is released correcting just a couple of things but keeping the schema the same, we have no updates needed on our side. The schema just ensures that the string is an actual semver string.
2 participants
Closes #82
Open Q: do we want to label output files with this version as we do with some of the others? It's unnecessary right now as we limit it to a single version.
Reviewer checklist
pipdependencies in the module's environment files (workflow/envs/).pathvars(e.g.,<results>) in their inputs and outputs.pre-commit.citests pass.INTERFACE.yamlmentions all relevantpathvarsandwildcards.README.mddescribes how to use the module and has the necessary citations.