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 DataArray.gmt.imshow() function to show 2D DataArray easily #2372

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

seisman
Copy link
Member

@seisman seisman commented Feb 17, 2023

Description of proposed changes

Related to #1412.

Fixes #

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@seisman seisman added the feature Brand new feature label Feb 18, 2023
>>> grid.gmt.imshow()
"""
fig = Figure()
fig.grdimage(self._obj, frame=True)
Copy link
Member

Choose a reason for hiding this comment

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

Should we do imshow(self, *kwargs) and allow users to pass extra keyword arguments to grdimage?

"""
fig = Figure()
fig.grdimage(self._obj, frame=True)
fig.colorbar()
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little on the fence about adding a colorbar. Sometimes the default colorbar looks very bad (e.g. too many black lines). Do you think we should:

  1. Add a setting to imshow like show_colorbar=True to enable/disable the colorbar if needed?
  2. Allow extra keyword arguments to colorbar somehow, so that users can tweak it if needed?

Or would this just not show a colorbar by default? What do others think?

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'm a little on the fence about adding a colorbar. Sometimes the default colorbar looks very bad (e.g. too many black lines).

That's because the original CPT file has too many z-slices. fig.colorbar(frame=True) should always be good.

fig = Figure()
fig.grdimage(self._obj, frame=True)
fig.colorbar()
fig.show()
Copy link
Member

Choose a reason for hiding this comment

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

Debating on whether we should add return fig, so that users could potentially add extra stuff to the figure like so:

fig = grid.gmt.imshow()
fig.colorbar()
fig.show()

or would that get a bit confusing? I'm thinking a bit about how matplotlib does it where users can add extra elements to the figure, but unsure if we want to mimic that in PyGMT.

Copy link
Member Author

Choose a reason for hiding this comment

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

See my comment in #1412 (comment).

Initially, I propose to add the imshow() function as a quick DataArray viewer, so I expect to view the grid using a simple single command.

Similar to the ax parameter in xarray.DataArray.plot.imshow, I think the imshow function can at least accept a fig parameter.

If fig=None (the default behavior), grid.gmt.imshow() should plot the grid and show the image.

If a Figure instance is given, then we know that users want to plot the grid in an existing figure, then we should not show the image.

Then the imshow() will be like:

    def imshow(self, fig=None):
        if fig is None:
            fig = Figure()

        fig.grdimage(self._obj, frame=True)
        fig.colorbar()

        if fig is None:
            fig.show()
        else:
            return fig

@seisman seisman added this to the 0.12.0 milestone Dec 11, 2023
@seisman seisman removed this from the 0.12.0 milestone Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Brand new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants