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

Wrap makecpt #329

Merged
merged 7 commits into from
Oct 10, 2019
Merged

Wrap makecpt #329

merged 7 commits into from
Oct 10, 2019

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Sep 28, 2019

Description of proposed changes

Wrapping the makecpt function! Tentatively implemented here under the mathops.py file because it is classfied under "Mathematical operations on tables or grids" in GMT upstream, but I've placed the documentation for makecpt under 'Plotting' for now.

Example code of how it works:

import pygmt
fig = pygmt.Figure()
pygmt.makecpt(cmap="oleron", series="-4500/4500")
fig.grdimage(grid=pygmt.datasets.load_earth_relief(), projection="W0/6i")
fig.show()

Produces:

PyGMT makecpt to change grid colour to oleron

Cross-referencing Julia wrapper/implementation at https://www.generic-mapping-tools.org/GMT.jl/latest/#GMT.makecpt.

Parameters/Aliases to wrap:

  • A = alpha/transparency
  • C = cmap (or color in GMT.jl)
  • D
  • E = data_levels
  • F = force_rgb
  • G = truncate
  • H = output
  • I = reverse (or inverse in GMT.jl)
  • M = overrule_bg
  • N = no_bg/nobg
  • Q = log
  • S = auto
  • T = series (or range in GMT.jl)
  • W = wrap/categorical
  • Z = continuous
    ...

Fixes #214

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 adding new functionality, add an example to docstrings or tutorials.

Initial commit for wrapping the makecpt function raised at #214, tentatively implemented here under the mathops.py file which will hold functions for "Mathematical operations on tables or grids", but with documentation placed under 'Plotting'. Original GMT `makecpt` documentation can be found at https://docs.generic-mapping-tools.org/latest/makecpt.html.

Tests are stored under test_makecpt.py, and there are now some basic tests ensuring that we can change the color when plotting points and grids. Current implementation uses 'cmap' and 'series' as aliases for 'C' and 'T' respectively.
@joa-quim
Copy link
Member

Hi @weiji14, it would be nice if we had more coordination with the Julia wrapper. Specially in what regards the keyword names. For example, looking at your example I believe you named the option -T series, while I called it range. The julia wrapper lacks lots of documentation (it's a huge task) but the comment section of each module has the names given to all options. And I have done that to all modules (but the grd|gmt math ones). Those names can be still changed or added more aliases, so any discussion about them is completely open.

@weiji14
Copy link
Member Author

weiji14 commented Sep 28, 2019

Funny you said that, I did think of using ‘range’ at one point (might have looked at your Julia implementation actually) but unfortunately that’s a reserved function in Python so had to come up with another name. Other than that though, I do agree that we should keep things consistent across implementations.

Might be good to track this upstream, perhaps at GenericMappingTools/gmt#230? That way we don’t keep reinventing the wheel/aliases each time.

As a side note, if you need help with documentation, there’s this hacktoberfest thing coming up that would be terrific for encouraging more contributions.

@joa-quim
Copy link
Member

Ah, yes range is that keyword used often in loops. I had the same issue with the keyword end that is also reserved in Julia.

The long-format GMT issue is the right place but when we go ahead of it we must baptize the keywords anyway.

Thanks for the hacktoberfest link. It just got me in a period of short free time but I'll try to get in, even if not right from beginning of October.

Allow makecpt to save the generated color palette table to a file via the output (H) argument. The 'H' setting in GMT upstream (see https://docs.generic-mapping-tools.org/latest/makecpt.html#h) is actually a flag to force the creation of an output (to stdout) in modern mode. Here we use it to set the filename too.

See also GenericMappingTools/gmt#827 and GenericMappingTools/gmt#823
With rainbow cmap checks to test various zlow/zhigh combinations.
Used 'earth' cmap to test various reversed colormap examples.
@weiji14 weiji14 requested a review from a team October 4, 2019 02:57
@weiji14
Copy link
Member Author

weiji14 commented Oct 4, 2019

I think this is ready for review. The cmap (C) and series (T) aliases should pretty much cover the most basic needs, and with truncate (G) and reverse (I) that should cover most use-cases (at least the ones I can think of). The output (H) functionality is quite different from upstream GMT. Here I've set it to take a file name, instead of having it produce an output to 'stdout' in modern mode.

Happy to consider any other suggestions too 😄

@weiji14 weiji14 added this to the 0.1.0 milestone Oct 4, 2019
Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

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

It looks good to me.

Can you also implement the -Z option? I believe it's also commonly used.

pygmt/mathops.py Outdated Show resolved Hide resolved
outfile = kwargs.pop("H")
if not outfile or not isinstance(outfile, str):
raise GMTInvalidInput("'output' should be a proper file name.")
arg_str = " ".join([build_arg_string(kwargs), f"-H > {outfile}"])
Copy link
Member

Choose a reason for hiding this comment

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

Other functions in PyGMT which output to a file use -> instead of >. I don't know why but maybe you can try if -> also works.

Suggested change
arg_str = " ".join([build_arg_string(kwargs), f"-H > {outfile}"])
arg_str = " ".join([build_arg_string(kwargs), f"-H -> {outfile}"])

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 pretty sure it'll work, and I did notice it in other parts of the code but never quite understood why -> is used. Is it meant to be a cross-platform thing?

Copy link
Member Author

@weiji14 weiji14 Oct 7, 2019

Choose a reason for hiding this comment

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

Ok, I was wrong, -> doesn't work. The test_makecpt_output_to_cpt_file function fails with this error message:

    def test_makecpt_output_to_cpt_file():
        """
        Save the generated static color palette table to a .cpt file
        """
        with GMTTempFile(suffix=".cpt") as cptfile:
>           makecpt(output=cptfile.name)

../pygmt/tests/test_makecpt.py:82: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../pygmt/helpers/decorators.py:187: in new_module
    return module_func(*args, **kwargs)
../pygmt/helpers/decorators.py:282: in new_module
    return module_func(*args, **kwargs)
../pygmt/mathops.py:65: in makecpt
    lib.call_module(module="makecpt", args=arg_str)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <pygmt.clib.session.Session object at 0x7fe9ae6ad470>, module = 'makecpt', args = ' -H -> /tmp/pygmt-dz2kwkqs.cpt'

    def call_module(self, module, args):
        """
        Call a GMT module with the given arguments.
    
        Makes a call to ``GMT_Call_Module`` from the C API using mode
        ``GMT_MODULE_CMD`` (arguments passed as a single string).
    
        Most interactions with the C API are done through this function.
    
        Parameters
        ----------
        module : str
            Module name (``'pscoast'``, ``'psbasemap'``, etc).
        args : str
            String with the command line arguments that will be passed to the
            module (for example, ``'-R0/5/0/10 -JM'``).
    
        Raises
        ------
        GMTCLibError
            If the returned status code of the function is non-zero.
    
        """
        c_call_module = self.get_libgmt_func(
            "GMT_Call_Module",
            argtypes=[ctp.c_void_p, ctp.c_char_p, ctp.c_int, ctp.c_void_p],
            restype=ctp.c_int,
        )
    
        mode = self["GMT_MODULE_CMD"]
        status = c_call_module(
            self.session_pointer, module.encode(), mode, args.encode()
        )
        if status != 0:
            raise GMTCLibError(
                "Module '{}' failed with status code {}:\n{}".format(
>                   module, status, self._error_message
                )
            )
E           pygmt.exceptions.GMTCLibError: Module 'makecpt' failed with status code 71:
E           makecpt [ERROR]: Syntax error: No input files expected unless -E or -S are used

../pygmt/clib/session.py:490: GMTCLibError
--------------------------------------------------------------------------------- Captured stderr call ---------------------------------------------------------------------------------
makecpt [ERROR]: Syntax error: No input files expected unless -E or -S are used

Included one test to create a continuous cpt from blue to white. Also updated link to the full list of GMT's color palette tables due to documentation reorganization during GenericMappingTools/gmt#1594.
@weiji14 weiji14 mentioned this pull request Oct 8, 2019
5 tasks
@weiji14 weiji14 requested a review from seisman October 10, 2019 02:35
@weiji14 weiji14 merged commit fabc9d2 into GenericMappingTools:master Oct 10, 2019
@weiji14 weiji14 deleted the mathops/makecpt branch October 10, 2019 03:29
weiji14 added a commit to weiji14/deepbedmap that referenced this pull request Oct 29, 2019
@weiji14 weiji14 mentioned this pull request Apr 10, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrapper for makecpt
3 participants