Report data file errors in more detail #1782
Conversation
4c63e61
to
49fcc01
Compare
8cdb7d8
to
a6beae3
Compare
|
Thanks for doing this. I would like to include the "coverage combine" message only if there are combinable data files. I've included other comments in the diff. |
| for filename in combinable_files(data_file): | ||
| print("-----") | ||
| debug_data_file(filename) | ||
| with suppress(NoDataFilesFoundError): |
There was a problem hiding this comment.
Why do we need to suppress this error here?
There was a problem hiding this comment.
We do not, but I tried to limit how many outputs I changed. Would you like this to display a friendly error instead?
| @@ -317,7 +317,9 @@ def report(self, morfs: Iterable[TMorf] | None) -> float: | |||
| file_be_gone(os.path.join(self.directory, ftr.html_filename)) | |||
|
|
|||
| if not have_data: | |||
| raise NoDataError("No data to report.") | |||
| raise DataFileOrDirectoryNotFoundError.new_for_data_file_or_directory( | |||
| os.path.dirname(self.coverage.get_data().base_filename()) | |||
There was a problem hiding this comment.
I have to think through whether the message here is correct. Is it always that the file doesn't exist?
There was a problem hiding this comment.
It's good that the exceptions centralize the messages, but everywhere this exception is created it uses the same fairly complex expression. Can we centralize that as well?
tests/test_api.py
Outdated
| with pytest.raises( | ||
| NoDataError, | ||
| match=( | ||
| r"^The data file or directory `(.+?)` could not be found\. Perhaps `coverage " |
There was a problem hiding this comment.
"The data file or directory" seems odd to me. Don't we know whether we are expecting a file or directory?
There was a problem hiding this comment.
These results depend on the paths entered in the CLI. As far as I understand the code these are allowed to be data file paths or paths to directories containing data files. If the path does not exist, I'm not sure we would know if the user meant a file or a directory.
…ths, and make helpful suggestions
a6beae3
to
8bf5006
Compare
This PR fixes #1781
Changes
NoDataErrorsubclasses for specific purposes