GenericMappingTools / GenericMappingTools/pygmt
Add new exceptions for invalid inputs and deprecate GMTInvalidInput exception
- Dominant language
- Python
- Stars
- 874
- Forks
- 255
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 40
Description
Currently, we have several exceptions defined in https://github.com/GenericMappingTools/pygmt/blob/main/pygmt/exceptions.py.
Below is a simple command to count how often each exception is raised in the codebase (The `tests` directory and the `exceptions.py` file are excluded).
```bash
for e in \
GMTError \
GMTOSError \
GMTCLibError \
GMTCLibNotFoundError \
GMTCLibNoSessionError \
GMTVersionError \
GMTInvalidInput \
GMTImageComparisonFailure;
do
n=$(grep -R --exclude-dir=tests --exclude="exceptions.py" "raise $e" pygmt | wc -l)
echo "$e: $n"
done
```
the output is:
```
GMTError: 1
GMTOSError: 1
GMTCLibError: 16
GMTCLibNotFoundError: 1
GMTCLibNoSessionError: 1
GMTVersionError: 1
GMTInvalidInput: 87
GMTImageComparisonFailure: 1
```
Most exceptions are about the GMT C library, and is only used once or a few times. But the `GMTInvalidInput` exception is used 87 times and will definitely increase as we wrap more modules.
The `GMTInvalidInput` exception is general and doesn't tell exactly what's happening. I haven't checked the source codes carefully, but I feel the exceptions can be grouped into a few categories:
1. A required parameter is not given
2. Given parameters have conflicts
3. A parameter is given with an incorrect type
4. A parameter is given with invalid values
5. Maybe more
I think we should have more accurate exceptions, e.g.,
- [x] `GMTParameterError`: for cases 1 and 2, maybe having more fine-grain exceptions like `GMTRequiredParameterError`/`GMTConflictParameterError`
- [x] `GMTTypeError`: for case 3, similar to the built-in `TypeError` #3999
- [x] `GMTValueError`: for case 4, similar to the built-in `ValueError` #3985
The exception may even start with `PYGMT`, not `GMT`, since these are not directly related to GMT.
Contributor guide
Assessment
This issue has not been assessed yet.