Skip to content

Conversation

@seisman
Copy link
Member

@seisman seisman commented Apr 16, 2024

Description of proposed changes

Workaround for #3170. A test is also added.

@seisman seisman force-pushed the dataset/none-string branch 2 times, most recently from 014561d to d6ffe1a Compare April 16, 2024 00:18
@seisman seisman force-pushed the dataset/none-string branch from d6ffe1a to 056fe4c Compare April 16, 2024 00:37
@seisman seisman added the maintenance Boring but important stuff for the core devs label Apr 16, 2024
@seisman seisman added this to the 0.12.0 milestone Apr 16, 2024
@seisman seisman added the needs review This PR has higher priority and needs review. label Apr 16, 2024
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

A couple of minor suggestions.

"It's likely caused by an upstream GMT API bug. "
"Please consider reporting to us."
)
warnings.warn(msg, category=RuntimeWarning, stacklevel=1)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
warnings.warn(msg, category=RuntimeWarning, stacklevel=1)
warnings.warn(msg, category=RuntimeWarning, stacklevel=2)

Copy link
Member Author

@seisman seisman Apr 16, 2024

Choose a reason for hiding this comment

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

stacklevel=1 makes more sense here.

stacklevel=1:

pygmt/datatypes/dataset.py:166: RuntimeWarning: The trailing text column contains `None' values and has been replace with an empty string to avoid TypeError exceptions. It's likely caused by an upstream GMT API bug. Please consider reporting to us.
  warnings.warn(msg, category=RuntimeWarning, stacklevel=1)

stacklevel=2:

pygmt/clib/session.py:1940: RuntimeWarning: The trailing text column contains `None' values and has been replace with an empty string to avoid TypeError exceptions. It's likely caused by an upstream GMT API bug. Please consider reporting to us.
  return result.to_strings()

@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 Apr 16, 2024
@seisman seisman merged commit 35ed27a into main Apr 16, 2024
@seisman seisman deleted the dataset/none-string branch April 16, 2024 04:04
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Apr 16, 2024
seisman added a commit that referenced this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Boring but important stuff for the core devs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants