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 placeholder codes for wrapping GMT_IMAGE/GMT_CUBE #3181

Closed
wants to merge 1 commit into from

Conversation

seisman
Copy link
Member

@seisman seisman commented Apr 19, 2024

I don't think we can finish wrapping GMT_IMAGE and GMT_CUBE in the next few weeks, so it is better to push them to PyGMT v0.13.0.

This PR cherry-pick codes used in wrapping GMT_IMAGE/GMT_CUBE (#3150 and #3128) to minimize potential conflicts in #3150 and #3128.

@seisman seisman added maintenance Boring but important stuff for the core devs needs review This PR has higher priority and needs review. labels Apr 19, 2024
@seisman seisman added this to the 0.12.0 milestone Apr 19, 2024
@seisman seisman requested a review from a team April 19, 2024 10:02
Comment on lines -1805 to +1814
raise NotImplementedError(f"kind={kind} is not supported yet.")
dtype = {"dataset": _GMT_DATASET, "grid": _GMT_GRID}[kind]
dtype = {
"dataset": _GMT_DATASET,
"grid": _GMT_GRID,
"image": _GMT_IMAGE,
"cube": _GMT_CUBE,
}[kind]
Copy link
Member

Choose a reason for hiding this comment

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

If we merge this PR for PyGMT v0.12.0, then NotImplementedError won't be raised right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but since it's a low-level API function, I guess users won't call it anyway and we maintainers should know what we're doing.

Copy link
Member

@weiji14 weiji14 Apr 19, 2024

Choose a reason for hiding this comment

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

But kind='image' and kind='cube' is showing up in the API docs at https://pygmt-dev--3181.org.readthedocs.build/en/3181/api/generated/pygmt.clib.Session.virtualfile_out.html. Can we push this PR to v0.13.0? There's no real benefit to users in having this merged anyway without GMT_IMAGE/GMT_CUBE being wrapped right?

Copy link
Member

Choose a reason for hiding this comment

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

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 guess we can close this PR, since it's likely we will finish wrapping GMT_IMAGE first and then GMT_CUBE later, so we only need to deal with the conflicts in PR #3150.

@seisman seisman closed this Apr 19, 2024
@seisman seisman deleted the image-cube-placeholder branch April 19, 2024 12:11
@seisman seisman removed the needs review This PR has higher priority and needs review. label Apr 19, 2024
@seisman seisman removed this from the 0.12.0 milestone Apr 19, 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.

2 participants