Improve handling of missing file requirements in readers #452
Conversation
mraspaud
added
enhancement
Codecov Report
@@ Coverage Diff @@
## master #452 +/- ##
=========================================
+ Coverage 76.81% 77.02% +0.2%
=========================================
Files 136 136
Lines 19029 19076 +47
=========================================
+ Hits 14618 14694 +76
+ Misses 4411 4382 -29
Continue to review full report at Codecov.
|
satpy/scene.py
Outdated
| @@ -150,6 +150,12 @@ def __init__(self, filenames=None, reader=None, filter_parameters=None, reader_k | |||
| self.readers = self.create_reader_instances(filenames=filenames, | |||
| reader=reader, | |||
| reader_kwargs=reader_kwargs) | |||
|
|
|||
| if filenames and not self.available_dataset_ids(reader_name=reader): | |||
There was a problem hiding this comment.
The only problem I could not resolve is: If no file matches the filter_parameters the exception is triggered mistakenly. But I would expect create_reader_instances to throw an exception in this case? Similar to No supported files found raised by find_files_and_readers. At the moment you would run into the Start time unknown until files are selected error.
There was a problem hiding this comment.
Sorry I'm a little confused. So if none of the files match (due to filter_parameters or other reasons) then create_reader_instances isn't raising an exception or it is?
Also, you should probably remove the reader_name=reader part since filenames can be a dictionary mapping reader: [filenames]. Just to check if there are any available datasets for any possible readers.
There was a problem hiding this comment.
create_reader_instances does not raise an exception in this case. Instead the subsequent call to _compute_metadata_from_readers causes the following exception:
Traceback (most recent call last):
File "test.py", line 21, in <module>
end_time=datetime.datetime(2010, 1, 2)
File "/cmsaf/nfshome/sfinkens/software/devel/satpy/satpy/scene.py", line 159, in __init__
self.attrs.update(self._compute_metadata_from_readers())
File "/cmsaf/nfshome/sfinkens/software/devel/satpy/satpy/scene.py", line 177, in _compute_metadata_from_readers
for x in self.readers.values())
File "/cmsaf/nfshome/sfinkens/software/devel/satpy/satpy/scene.py", line 177, in <genexpr>
for x in self.readers.values())
File "/cmsaf/nfshome/sfinkens/software/devel/satpy/satpy/readers/yaml_reader.py", line 296, in start_time
raise RuntimeError("Start time unknown until files are selected")
RuntimeError: Start time unknown until files are selected
There was a problem hiding this comment.
So maybe load_readers (in satpy/readers/__init__.py which is called by create_reader_instances) should change:
if not reader_instances:
raise ValueError("No supported files found")to something like:
if not reader_instances or all(reader.available_dataset_ids for reader in reader_instances):
raise ValueError("No supported files found")Then I think you could remove the scene.py change. If the error messages would be clearer I suppose the above if statement could be split in to if not reader_instances: and elif all(...): so that it is more clear "you gave valid files, but not enough, or not useable ones" or something like what you had.
There was a problem hiding this comment.
Good idea! I moved the check & exception to load_readers
There was a problem hiding this comment.
You said you moved this, but it still exists and doesn't seem to exist in load_readers. Forgot to push something?
There was a problem hiding this comment.
Strange, my local and remote branch are up-to-date. Also, the Files Changed tab shows the diffs I commited, especially no changes to scene.py. Maybe if I fix the typo and push again, it will be updated...
satpy/scene.py
Outdated
| @@ -150,6 +150,12 @@ def __init__(self, filenames=None, reader=None, filter_parameters=None, reader_k | |||
| self.readers = self.create_reader_instances(filenames=filenames, | |||
| reader=reader, | |||
| reader_kwargs=reader_kwargs) | |||
|
|
|||
| if filenames and not self.available_dataset_ids(reader_name=reader): | |||
There was a problem hiding this comment.
You said you moved this, but it still exists and doesn't seem to exist in load_readers. Forgot to push something?
djhoese
changed the title
When the exception is catched, redirect the message into the warning. To improve readability, make the warning statement fit in one line and don't exclude the source line from the message.
BaseException.message was removed in python3
This reverts commit d185699.
This reverts commit 1f5cce9.
This reverts commit 177593d.
This reverts commit 5a46baf.
If valid files were given but no dataset could be loaded, raise an exception.
1) There is no handler for reading a requirement 2) There is a handler but it doesn't match the current file
djhoese
force-pushed
the
fix-missing-requirements
branch
from
4947873
to
9b9e725
Compare
The hrit_msg reader is now called seviri_l1b_hrit
Improve the warning about a missing requirement (e.g. Epilogue, Prologue) and abort if there are insufficient requirements to read any data.
RuntimeErrorif there are insufficient requirements to read any data -> No moreStart time unknown until files are selectederrors. In particular it ensures a consistent exception ifThe modifications still allow to read as much data as possible. E.g. missing requirements for one out of 10 images will trigger warnings, but no exceptions.
Example output:
On-the-fly decompression is addressed by #436.