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

pygmt.grd2cpt & pygmt.makecpt: Simplify the logic for dealing with CPT output #3334

Merged
merged 3 commits into from
Jul 19, 2024

Conversation

seisman
Copy link
Member

@seisman seisman commented Jul 17, 2024

Simplify:

        if kwargs.get("H") is None:  # if no output is set
            arg_str = build_arg_list(kwargs)
        else:  # if output is set
            outfile, kwargs["H"] = kwargs.pop("H"), True
            if not outfile or not isinstance(outfile, str):
                raise GMTInvalidInput("'output' should be a proper file name.")
            arg_str = build_arg_list(kwargs, outfile=outfile)

to

    if (output := kwargs.pop("H", None)) is not None:
        if not isinstance(output, str) or output == "":
            raise GMTInvalidInput("'output' should be a proper file name.")
        kwargs["H"] = True
    arg_str = build_arg_list(kwargs, outfile=output)

It works because outfile in build_arg_list can be None.

@seisman seisman added the maintenance Boring but important stuff for the core devs label Jul 17, 2024
@seisman seisman added this to the 0.13.0 milestone Jul 17, 2024
@seisman seisman added needs review This PR has higher priority and needs review. run/benchmark Trigger the benchmark workflow in PRs labels Jul 17, 2024
pygmt/src/grd2cpt.py Outdated Show resolved Hide resolved
pygmt/src/makecpt.py Outdated Show resolved Hide resolved
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Needed to read that if-statement a few times to get the logic, but looks ok.

@seisman
Copy link
Member Author

seisman commented Jul 19, 2024

The if-statement can be further simplified after PR #3238

@seisman seisman merged commit 6c436a3 into main Jul 19, 2024
22 checks passed
@seisman seisman deleted the cpt-wrapper branch July 19, 2024 02:30
@seisman seisman removed needs review This PR has higher priority and needs review. run/benchmark Trigger the benchmark workflow in PRs labels Jul 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