Skip to content

Conversation

seisman
Copy link
Member

@seisman seisman commented Sep 3, 2025

This PR improves the style parameter (i.e., -S option). Previously the style parameter can only accept l/n/u. Now it can accept more descriptive arguments standard/no_label/url.

The upstream GMT uses long names standard/none/url (see https://github.com/GenericMappingTools/gmt/blob/c95470b1c78c412fa15d388be0d9b07de111ba49/src/longopt/gmtlogo_inc.h#L37), but I feel no_label is more descriptive.

Preview: https://pygmt-dev--4074.org.readthedocs.build/en/4074/api/generated/pygmt.Figure.logo.html

@seisman seisman added this to the 0.17.0 milestone Sep 3, 2025
@seisman seisman requested review from a team and Copilot September 3, 2025 14:23
@seisman seisman added enhancement Improving an existing feature needs review This PR has higher priority and needs review. labels Sep 3, 2025
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 enhances the style parameter in the Figure.logo method to accept more descriptive string arguments instead of single-letter codes. The change improves code readability by allowing "standard", "url", and "no_label" instead of cryptic "l", "u", and "n" values.

  • Updated the style parameter to use descriptive literal types with proper type hints
  • Implemented alias mapping to convert descriptive names to GMT's single-letter codes
  • Updated documentation to reflect the new descriptive parameter values

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@weiji14
Copy link
Member

weiji14 commented Sep 3, 2025

Hmm, I'm getting a 403 when trying to access https://dagshub.com/GenericMappingTools/pygmt, server might be down or something.

@seisman
Copy link
Member Author

seisman commented Sep 4, 2025

Hmm, I'm getting a 403 when trying to access https://dagshub.com/GenericMappingTools/pygmt, server might be down or something.

I can access the website, and I can even run dvc pull successfully using my local computer. So it's more likely CI issues.

@yvonnefroehlich
Copy link
Member

Hmm, I'm getting a 403 when trying to access https://dagshub.com/GenericMappingTools/pygmt, server might be down or something.

I can access the website, and I can even run dvc pull successfully using my local computer.

Same for me.

@yvonnefroehlich
Copy link
Member

The upstream GMT uses long names standard/none/url (see https://github.com/GenericMappingTools/gmt/blob/c95470b1c78c412fa15d388be0d9b07de111ba49/src/longopt/gmtlogo_inc.h#L37), but I feel no_label is more descriptive.

I prefere no_label. In general, I feel having "none" and None is not optimal.

seisman and others added 3 commits September 4, 2025 18:29
Co-authored-by: Yvonne Fröhlich <[email protected]>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.4.3 to 5.5.0.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@18283e0...fdcc847)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-version: 5.5.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@seisman
Copy link
Member Author

seisman commented Sep 6, 2025

I tried debugging the DVC issue but wasn't successful. I think we'll have to move on for now and revisit it in the future if a fix becomes available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving an existing feature needs review This PR has higher priority and needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants