Skip to content

Update Ruff pre-commit hook to v0.3.4#16264

Merged
astrofrog merged 1 commit into
astropy:mainfrom
eerovaher:ruff-034
Apr 4, 2024
Merged

Update Ruff pre-commit hook to v0.3.4#16264
astrofrog merged 1 commit into
astropy:mainfrom
eerovaher:ruff-034

Conversation

@eerovaher
Copy link
Copy Markdown
Member

@eerovaher eerovaher commented Apr 3, 2024

Description

The Ruff rule UP032 can automatically convert str.format() calls to f-strings. Before v0.3.2 Ruff avoided making changes if the naive conversion produced lines that exceeded the line length limit (88 characters for us), but now it is more aggressive. Updating the pre-commit hook manually prevents Ruff from producing overlong lines like it tried to do in #16253.

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@eerovaher
Update Ruff pre-commit hook to v0.3.4
f9d233f 
Ruff rule UP032 can automatically convert `str.format()` calls to
f-strings. Before v0.3.2 Ruff avoided making changes if the naive
conversion produced lines that exceeded the line length limit, but now
it is more aggressive. The `pre-commit` hook is updated manually to
prevent Ruff from producing overlong lines.
@eerovaher eerovaher added this to the v7.0.0 milestone Apr 3, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 3, 2024

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@pllim
Copy link
Copy Markdown
Member

pllim commented Apr 3, 2024

Thanks! I see that you milestoned this to v7.0.0, which means we cannot merge until v6.1.x is branched out.

taldcroft
taldcroft approved these changes Apr 3, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

Good for io.ascii, time, table.

@neutrinoceros
Copy link
Copy Markdown
Contributor

neutrinoceros commented Apr 3, 2024

I see that you milestoned this to v7.0.0, which means we cannot merge until v6.1.x is branched out.

Wouldn't this also mean that all bot PRs will be noise until this one is merged ?

mhvk
mhvk approved these changes Apr 3, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

units and utils are fine (not much of an improvement, but also not really worse). Not a rule I feel we should necessarily enforce

astrofrog
astrofrog approved these changes Apr 4, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@astrofrog astrofrog 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 to me and will merge to include in v6.1.0 to reduce backport issues.

@astrofrog astrofrog modified the milestones: v7.0.0, v6.1.0 Apr 4, 2024
@astrofrog astrofrog enabled auto-merge April 4, 2024 10:20
@astrofrog astrofrog disabled auto-merge April 4, 2024 10:31
@astrofrog astrofrog merged commit f294168 into astropy:main Apr 4, 2024
@dhomeier
Copy link
Copy Markdown
Contributor

dhomeier commented Apr 4, 2024

units and utils are fine (not much of an improvement, but also not really worse). Not a rule I feel we should necessarily enforce

Agreed; should perhaps discuss how to deal with cases like
"a {0:s} b {0:s}c{1:s} 1 {0:s}'2'{0:s} 3.0".format(delimiter, eol)
where .format() is obviously the more efficient construct.

@eerovaher eerovaher deleted the ruff-034 branch April 4, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@astrofrog astrofrog astrofrog approved these changes

@taldcroft taldcroft taldcroft approved these changes

@mhvk mhvk mhvk approved these changes

@nstarman nstarman nstarman approved these changes

@WilliamJamieson WilliamJamieson Awaiting requested review from WilliamJamieson WilliamJamieson is a code owner

@Cadair Cadair Awaiting requested review from Cadair

@saimn saimn Awaiting requested review from saimn saimn is a code owner

@dhomeier dhomeier Awaiting requested review from dhomeier dhomeier is a code owner

Assignees

No one assigned

Labels

coordinates dev-automation io.ascii io.fits io.votable nddata no-changelog-entry-needed samp skip-changelog-checks Tells bot to skip changlog checks table testing time units utils.iers utils wcs.wcsapi

Projects

None yet

Milestone

v6.1.0

Development

Successfully merging this pull request may close these issues.

8 participants

@eerovaher @pllim @neutrinoceros @dhomeier @astrofrog @taldcroft @mhvk @nstarman