Skip to content

Conversation

@sjvrijn
Copy link
Contributor

@sjvrijn sjvrijn commented Sep 8, 2025

Checklist before requesting a review

  • I have read the contribution guidelines
  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • [ ] All user facing changes have been added to CHANGELOG.md

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

List of related issues or pull requests**

Refs:

Describe the changes made in this pull request

Removed the reference to yapf for formatting code, and replaced it with an instruction to use ruff format instead.

@sjvrijn sjvrijn requested a review from fdiblen September 8, 2025 10:37
@sjvrijn sjvrijn force-pushed the 588-replace-yapf-with-ruff-format branch 3 times, most recently from 0d20fc2 to 2d2f525 Compare September 9, 2025 13:59
@sjvrijn sjvrijn force-pushed the 588-replace-yapf-with-ruff-format branch from 2d2f525 to ab31a4b Compare November 16, 2025 16:30
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Nice work, making things more consistent.

In README.dev.md the ruff commands are shown with ..
In template/{% if AddPreCommit %}.githooks{% endif %}/pre-commit and template/{% if HasWorkflows %}.github{% endif %}/workflows/{% if AddGitHubActionBuild %}build.yml{% endif %}.jinja the ruff commands are run without ..

Would be nice to be consistent with the commands by either using ruff check . or ruff check everywhere.

nitpick

When I do ruff format . I get

warning: The following rules have been removed and ignoring them has no effect:
    - ANN101
    - ANN102

warning: The following rule may cause conflicts when used with the formatter: `COM812`. To avoid unexpected behavior, we recommend disabling this rule, either by removing it from the `lint.select` or `lint.extend-select` configuration, or adding it to the `lint.ignore` configuration.
4 files left unchanged

Might be nice to get rid of those warnings. This is with ruff v0.14.5.

@sjvrijn
Copy link
Contributor Author

sjvrijn commented Nov 17, 2025

@sverhoeven Thanks for the suggestions! I've updated the ruff calls to use . explicitly, and I've included COM812 (missing trailing commas) in the ignore list, since ruff format already fixes that.

The warnings on ANN101 & 102 were already removed in a previous PR, I think you got those because copier uses the latest tag/release by default. My bad for not the relevant copier command to use when testing to the PR in the first place 😅

copier copy --vcs-ref 588-replace-yapf-with-ruff-format https://github.com/nlesc/python-template test_package

@sjvrijn sjvrijn merged commit 6bed2db into main Nov 17, 2025
18 checks passed
@sjvrijn sjvrijn deleted the 588-replace-yapf-with-ruff-format branch November 17, 2025 10:41
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.

3 participants