Gracefully handle the case of missing facets required by esmvalcore.io.local.LocalDataSource#3021
Gracefully handle the case of missing facets required by esmvalcore.io.local.LocalDataSource#3021bouweandela wants to merge 7 commits intomainfrom
esmvalcore.io.local.LocalDataSource#3021Conversation
bouweandela
changed the title
esmvalcore.io.local.LocalDataSourceesmvalcore.io.local.LocalDataSource
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3021 +/- ##
=======================================
Coverage 95.70% 95.70%
=======================================
Files 267 267
Lines 15768 15792 +24
=======================================
+ Hits 15090 15114 +24
Misses 678 678 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
schlunma
left a comment
schlunma
left a comment
There was a problem hiding this comment.
Great, thanks Bouwe! Works exactly as expected.
I just have some minor comments on the code.
esmvalcore/io/local.py
Outdated
| class MissingFacetError(KeyError): | ||
| """Error raised when a facet required for filling the template is missing.""" |
There was a problem hiding this comment.
Any particular reason why you're not putting this into esmvalcore.exceptions?
There was a problem hiding this comment.
It is intended for internal use in this module. I made it private in c8d9225.
There was a problem hiding this comment.
Is there any public function that raises this exception? I find it always nice when packages make exceptions public in order to catch them.
There was a problem hiding this comment.
Good point, it's also used by the (deprecated) functionality to get the preprocessor output file name in esmvalcore.local. Let me make sure it's caught there too.
There was a problem hiding this comment.
Solved in c64c373. There are no more functions left that do not catch the exception if it is raised.
There was a problem hiding this comment.
Are you sure? It looks like _get_glob_patterns is still used in 3 other places (partly deprecated), which can probably be accessed with public functions:
- https://github.com/ESMValGroup/ESMValCore/blob/relax-local-data-source-facet-requirement/esmvalcore/local.py#L167
- https://github.com/ESMValGroup/ESMValCore/blob/relax-local-data-source-facet-requirement/esmvalcore/local.py#L276
- https://github.com/ESMValGroup/ESMValCore/blob/relax-local-data-source-facet-requirement/esmvalcore/cmor/_fixes/icon/_base_fixes.py#L333
There was a problem hiding this comment.
Should be fixed now..
| glob_patterns.extend( | ||
| data_source._get_glob_patterns(**self.extra_facets), # noqa: SLF001 | ||
| ) | ||
| possible_grid_paths = [d.parent / grid_name for d in glob_patterns] | ||
| for grid_path in possible_grid_paths: | ||
| if grid_path.is_file(): | ||
| logger.debug("Using ICON grid file '%s'", grid_path) | ||
| return self._load_cubes(grid_path) |
There was a problem hiding this comment.
For some reason, this does not work (ICON grid files are not found anymore). Code looks ok, don't really know why.
Maybe just try/except the exception similar to the other changes? It's probably not worth it to investigate this in detail..
There was a problem hiding this comment.
But then the issue would not be solved for the ICON project. Do you have a recipe I can run on Levante to test?
2 participants
Description
Do not raise an exception if a facet required by
esmvalcore.io.local.LocalDataSourceis missing, but log the problem and continue. Maybe the data can be found using some other data source and if not, the user will see the log message.Closes #2995
Link to documentation:
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.
To help with the number pull requests: