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

ruff: Fix Python 3.10 violations #3040

Merged
merged 4 commits into from
Feb 6, 2024
Merged

ruff: Fix Python 3.10 violations #3040

merged 4 commits into from
Feb 6, 2024

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Feb 5, 2024

Description of proposed changes

Fixes the following ruff violations with Python 3.10+

Addresses #3039 (comment)

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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash command is:

  • /format: automatically format and lint the code

@weiji14 weiji14 added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog labels Feb 5, 2024
@weiji14 weiji14 self-assigned this Feb 5, 2024
@weiji14 weiji14 changed the title ruff: Fix ruff: Fix Python 3.10 violations Feb 5, 2024
@seisman seisman added this to the 0.12.0 milestone Feb 5, 2024
@weiji14
Copy link
Member Author

weiji14 commented Feb 6, 2024

[ ] UP038 Use X | Y in isinstance call instead of (X, Y) - https://docs.astral.sh/ruff/rules/non-pep604-isinstance (should we ignore this, because the docs says X | Y might be slower than (X, Y)?)

Do we want to ignore UP038, or change to isinstance(..., X | Y)? I can run some benchmarks to see if it's noticeably slower. Edit: Benchmarks at https://codspeed.io/GenericMappingTools/pygmt/branches/ruff/python-3.10

@weiji14 weiji14 added the run/benchmark Trigger the benchmark workflow in PRs label Feb 6, 2024
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.

Looks good.

@weiji14
Copy link
Member Author

weiji14 commented Feb 6, 2024

Wanted to double-check the flame graphs, so I manually triggered the benchmarks workflow on the main branch to get the baseline (following steps at #2910 (comment)). Will merge this after I've had a look at the timing comparison.

@weiji14 weiji14 added run/benchmark Trigger the benchmark workflow in PRs and removed run/benchmark Trigger the benchmark workflow in PRs labels Feb 6, 2024
@weiji14
Copy link
Member Author

weiji14 commented Feb 6, 2024

Hmm, still not seeing the flame graph, but the performance benchmark didn't report the run as noticeably slower https://github.com/GenericMappingTools/pygmt/pull/3040/checks?check_run_id=21254596529, so should be ok.

@weiji14 weiji14 marked this pull request as ready for review February 6, 2024 06:20
@weiji14 weiji14 merged commit 31688c4 into main Feb 6, 2024
23 of 25 checks passed
@weiji14 weiji14 deleted the ruff/python-3.10 branch February 6, 2024 06:21
seisman pushed a commit to seisman/pygmt that referenced this pull request Feb 19, 2024
* Fix B905 [*] `zip()` without an explicit `strict=` parameter

Xref https://docs.astral.sh/ruff/rules/zip-without-explicit-strict

* Fix UP035 [*] Import from `collections.abc` instead: `Callable`

Xref https://docs.astral.sh/ruff/rules/deprecated-import

* Fix UP038 Use `X | Y` in `isinstance` call instead of `(X, Y)`

Xref https://docs.astral.sh/ruff/rules/non-pep604-isinstance
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 run/benchmark Trigger the benchmark workflow in PRs skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants