Skip to content

POC: Migrate the parameter 'verbose' to the new alias system and let it support descriptive arguments #4039

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

seisman
Copy link
Member

@seisman seisman commented Aug 8, 2025

This PR adds the .add_common method to the AliasSystem class, and also migrate the verbose parameter to the new alias system.

The .add_common method is necessary so that we don't have to repeat the following lines in all wrappers:

    aliasdict = AliasSystem(
        ...
        V=Alias(
            verbose,
            name="verbose",
            mapping={
                "quiet": "q",
                "error": "e",
                "warning": "w",
                "timing": "t",
                "information": "i",
                "compatibility": "c",
                "debug": "d",
            },
        ),
        ...
    )

instead, we just need to write like:

    aliasdict = AliasSystem(
        ....
    ).add_common(
        V=verbose,
    )
    aliasdict.merge(kwargs)

We can also write it with class method chaining,

    aliasdict = AliasSystem(
        J=Alias(projection, name="projection"),
    ).add_common(
        V=verbose,
    ).merge(kwargs)

but ruff will format it into an unreadable format like:

    aliasdict = (
        AliasSystem(
            J=Alias(projection, name="projection"),
        )
        .add_common(
            V=verbose,
        )
        .merge(kwargs)
    )

so, I prefer to write it like:

    aliasdict = AliasSystem(
        ....
    ).add_common(
        V=verbose,
    )
    aliasdict.merge(kwargs)

Will migrate other wrappers after approval.

@seisman seisman added this to the 0.17.0 milestone Aug 8, 2025
@seisman seisman added enhancement Improving an existing feature needs review This PR has higher priority and needs review. labels Aug 8, 2025
@seisman seisman requested a review from Copilot August 8, 2025 07:08
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the verbose parameter to the new alias system and introduces the .add_common() method to the AliasSystem class to reduce code duplication when handling common parameters across GMT wrappers.

  • Adds the .add_common() method to AliasSystem for centralized management of common parameters like verbose
  • Updates basemap function to use the new alias system for the verbose parameter with type hints and descriptive string arguments
  • Updates documentation to reflect the change from single-letter codes to descriptive argument names

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
pygmt/src/basemap.py Migrates verbose parameter to use new alias system with descriptive arguments and type hints
pygmt/alias.py Adds add_common method to AliasSystem class and updates docstring examples
doc/techref/common_parameters.md Updates documentation to use descriptive verbose argument names instead of single letters

seisman and others added 2 commits August 8, 2025 15:13
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Aug 10, 2025
@seisman seisman changed the title Migrate the parameter 'verbose' to the new alias system and let it support descriptive arguments POC: Migrate the parameter 'verbose' to the new alias system and let it support descriptive arguments Aug 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving an existing feature final review call This PR requires final review and approval from a second reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants