Skip to content

Conversation

@seisman
Copy link
Member

@seisman seisman commented Jul 15, 2025

Following PR #3985. Addressing #3707 and #3984.

This PR adds a new exception GMTTypeError. The error message is like:

Unrecognized data type: xxxx. Explain the reason.

@seisman seisman added this to the 0.17.0 milestone Jul 15, 2025
@seisman seisman added the enhancement Improving an existing feature label Jul 15, 2025
@seisman seisman requested a review from Copilot July 15, 2025 06:02
@seisman seisman added the needs review This PR has higher priority and needs review. label Jul 15, 2025
@seisman seisman marked this pull request as ready for review July 15, 2025 06:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the handling of invalid-type inputs by introducing and raising a new exception class GMTTypeError instead of GMTInvalidInput. Tests across many modules have been updated to expect GMTTypeError, and source files for core commands have been modified to import and raise the new exception. Additionally, the GMTTypeError class has been added to pygmt/exceptions.py.

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pygmt/exceptions.py Added GMTTypeError subclass of TypeError for invalid input types
pygmt/src/x2sys_cross.py Switched invalid-type error from GMTInvalidInput to GMTTypeError
pygmt/src/text.py Updated array‐argument checks to raise GMTTypeError
pygmt/src/plot3d.py Changed invalid‐array errors to raise GMTTypeError
pygmt/src/plot.py Changed invalid‐array errors to raise GMTTypeError
pygmt/src/legend.py Changed invalid‐type checks to raise GMTTypeError
pygmt/src/grdcut.py Switched unsupported‐type error to GMTTypeError
pygmt/clib/session.py Replaced several GMTInvalidInput raises with GMTTypeError, and updated docstrings
pygmt/tests/* Updated tests to import and expect GMTTypeError instead of GMTInvalidInput

@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Jul 21, 2025
@seisman
Copy link
Member Author

seisman commented Jul 21, 2025

Ping @GenericMappingTools/pygmt-maintainers for final reivews. I plan to merge in 48 hours.

Copy link
Member

@yvonnefroehlich yvonnefroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with the changes, just a few minor suggestions and comments.

raise GMTInvalidInput(msg)
raise GMTTypeError(
type(arg),
reason=f"Parameter {name!r} expects a scalar value or True.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With "scalar value", we refer to a float, or?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed it back to "single".

Co-authored-by: Yvonne Fröhlich <[email protected]>
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jul 21, 2025
@seisman
Copy link
Member Author

seisman commented Jul 21, 2025

/format

@seisman seisman merged commit eddc912 into main Jul 21, 2025
22 of 24 checks passed
@seisman seisman deleted the exception/typeerror branch July 21, 2025 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improving an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants