Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new exceptions for invalid inputs and deprecate GMTInvalidInput exception #3707

Open
seisman opened this issue Dec 23, 2024 · 0 comments
Labels
discussions Need more discussion before taking further actions

Comments

@seisman
Copy link
Member

seisman commented Dec 23, 2024

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).

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.,

  • GMTParameterError: for cases 1 and 2, maybe having more fine-grain exceptions like GMTRequiredParameterError/GMTConflictParameterError
  • GMTTypeError: for case 3, similar to the built-in TypeError
  • GMTValueError: for case 4, similar to the built-in ValueError.

The exception may even start with PYGMT, not GMT, since these are not directly related to GMT.

@seisman seisman added the discussions Need more discussion before taking further actions label Dec 23, 2024
@seisman seisman changed the title Add new exceptions for invalid inputs and deprecated GMTInvalidInput exception Add new exceptions for invalid inputs and deprecate GMTInvalidInput exception Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussions Need more discussion before taking further actions
Projects
None yet
Development

No branches or pull requests

1 participant