-
Notifications
You must be signed in to change notification settings - Fork 213
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
base: main
Are you sure you want to change the base?
Conversation
f14ea73
to
703eff0
Compare
>>> grid.gmt.imshow() | ||
""" | ||
fig = Figure() | ||
fig.grdimage(self._obj, frame=True) |
There was a problem hiding this comment.
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
?
pygmt/accessors.py
Outdated
""" | ||
fig = Figure() | ||
fig.grdimage(self._obj, frame=True) | ||
fig.colorbar() |
There was a problem hiding this comment.
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:
- Add a setting to
imshow
likeshow_colorbar=True
to enable/disable the colorbar if needed? - 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?
There was a problem hiding this comment.
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.
pygmt/accessors.py
Outdated
fig = Figure() | ||
fig.grdimage(self._obj, frame=True) | ||
fig.colorbar() | ||
fig.show() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Description of proposed changes
Related to #1412.
Fixes #
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version