-
Notifications
You must be signed in to change notification settings - Fork 224
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
Better handling of optional virtual files (e.g., shading in Figure.grdimage) #2493
Conversation
Currently, all the |
39af2d6
to
c024041
Compare
c024041
to
32c7303
Compare
1fb718f
to
c024577
Compare
59f326b
to
92bca38
Compare
92bca38
to
83a6360
Compare
@GenericMappingTools/pygmt-maintainers This PR is now completed and ready for review. |
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.
Nice work getting the if-then logic correct! Still a bit hard to follow in some parts, but I think this is the best that we can do. Just some more minor comments/suggestions and should be close to merge.
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
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.
Background
Some GMT modules (e.g.,
grdimage
andgrdview
) can accept a primary input file and an optional input file. Here, I'll takegrdimage
as an example.grdimage
can accept an input grid file and an optional intensity grid for shading (-I
option). Thus, in PyGMT,Figure.grdimage
'sshading
parameter can be given in many different ways:shading=None
: meaning no shadingshading=True
orshading=False
: shading or no shadingshading=0.5
: a constant intensity to apply everywhereshading="+a30+nt0.8"
: derive the intensity from the primary input gridshading="intensity.nc"
: an intensity grid fileshaidng="intensity.nc+a30+nt0.8"
: an intensity grid file plus more parametersshading=xrgrid
in whichxrgrid
is an xarray.DataArray objectFor cases 1-6, the
shading
parameter can be directly processed by thebuild_arg_string
function, but for case 7, we need to create a virtual file for the xarray.DataArray grid.Motivations
Since the virtual file for shading depends on the value and data type of the
shading
parameter, thus we have to use contextlib.ExitStack, which is designed to make it easy to work with optional context manager. However, usingcontextlib.ExitStack
still makes the codes complicated and also make it hard to reuse the codes:pygmt/pygmt/src/grdimage.py
Lines 182 to 195 in d2b0c39
The main goal of this PR is to simplify the above codes to:
which is more compact and more intuitive to read.
Implementations
To achieve this, the
virtualfile_from_data
context manager must work for all cases. It's actually very simple. Ifdata
is not an xarray.DataArray object (cases 1-6),virtualfile_from_data
should simply return a dummy context manager which does nothing but yield the argument passed to it (see thedummy_context
function but also see #2491). This is done by changing thedata_kind
function to take arguments like None, True, False, 0.5,"+a30+nt0.8"
,"intensity.nc+a30+nt0.8"
as a "file" (see the changes in thedata_kind
function).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