Skip to content

CI: bump pre commit not black#15998

Merged
nstarman merged 5 commits into
astropy:mainfrom
nstarman:ci-bump-pre-commit-not-black
Feb 14, 2024
Merged

CI: bump pre commit not black#15998
nstarman merged 5 commits into
astropy:mainfrom
nstarman:ci-bump-pre-commit-not-black

Conversation

@nstarman
Copy link
Copy Markdown
Member

@nstarman nstarman commented Feb 6, 2024 •
edited
Loading

Description

Partially addresses #15995.

  • 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.

@nstarman
bump pre-commit
7116810 
Signed-off-by: nstarman <[email protected]>
@nstarman
Apply updated pre-commit
d96ef6e 
Signed-off-by: nstarman <[email protected]>
@nstarman nstarman added this to the v6.1.0 milestone Feb 6, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 6, 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?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 6, 2024

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@nstarman nstarman force-pushed the ci-bump-pre-commit-not-black branch from d635092 to d8e8cfa Compare February 6, 2024 04:03
@nstarman
manual fixes
8b6b31c 
Signed-off-by: nstarman <[email protected]>
@nstarman
manual fix RUF017
67a106e 
Signed-off-by: nstarman <[email protected]>
@nstarman nstarman force-pushed the ci-bump-pre-commit-not-black branch from d8e8cfa to 67a106e Compare February 6, 2024 04:42
@nstarman nstarman marked this pull request as ready for review February 6, 2024 04:59
neutrinoceros
neutrinoceros approved these changes Feb 6, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

Thank you for doing this !

Comment thread .ruff.toml
eerovaher
eerovaher reviewed Feb 6, 2024
View reviewed changes
Comment thread .ruff.toml
"PERF401", # Use a list comprehension to create a transformed list

# pygrep-hooks (PGH)
"PGH001", # eval
Copy link
Copy Markdown
Member

@eerovaher eerovaher Feb 6, 2024

Choose a reason for hiding this comment

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

There is an ignore for this rule in pyproject.toml too:

astropy/pyproject.toml

Line 392 in 575eb8c

"PGH001", # No builtin eval() allowed

Copy link
Copy Markdown
Member Author

@nstarman nstarman Feb 7, 2024

Choose a reason for hiding this comment

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

✅

Comment thread astropy/coordinates/tests/test_frames.py Outdated
Comment thread astropy/timeseries/periodograms/lombscargle/implementations/fastchi2_impl.py Outdated
mhvk
mhvk reviewed Feb 6, 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.

Mostly looks good, certainly fine for units and utils.

Comment thread astropy/units/tests/test_quantity_non_ufuncs.py
Comment thread astropy/timeseries/periodograms/lombscargle/implementations/fastchi2_impl.py Outdated
Comment thread astropy/io/ascii/daophot.py Outdated
dhomeier
dhomeier requested changes Feb 6, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

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

I am with @mhvk on better getting by without chain in io.ascii. Applying RUF018 in the cases here and for consistency is fine with me.

Comment thread astropy/io/ascii/daophot.py Outdated
@nstarman
Address review comments
344c3cb 
Signed-off-by: nstarman <[email protected]>
@nstarman nstarman force-pushed the ci-bump-pre-commit-not-black branch from b76a115 to 344c3cb Compare February 7, 2024 20:11
mhvk
mhvk approved these changes Feb 7, 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.

Thanks, happy (enough) with this!

eerovaher
eerovaher approved these changes Feb 7, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@eerovaher eerovaher left a comment

Choose a reason for hiding this comment

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

I'd prefer the commits to be squashed, but all my suggestions about the final state of the patch have been addressed.

@nstarman
Copy link
Copy Markdown
Member Author

nstarman commented Feb 7, 2024

Squash-commit!

@pllim
Copy link
Copy Markdown
Member

pllim commented Feb 7, 2024

@dhomeier requested changes, so maybe he should have another look?

dhomeier
dhomeier approved these changes Feb 7, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

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

My requests were about the suggestions from @mhvk already included, just one question remaining for consistency in daophot remaining.
But this is acceptable either way, thanks!

Comment thread astropy/io/ascii/daophot.py
@eerovaher eerovaher mentioned this pull request Feb 13, 2024
Merged
1 task
@nstarman
Copy link
Copy Markdown
Member Author

nstarman commented Feb 14, 2024

Thanks all!

@nstarman nstarman merged commit 9a03dab into astropy:main Feb 14, 2024
@nstarman nstarman deleted the ci-bump-pre-commit-not-black branch February 14, 2024 01:43
d-giles referenced this pull request in d-giles/astropy-abdufork Jul 26, 2024
@nstarman @d-giles
CI: bump pre commit not black (#15998)
b4e23b1 
* bump pre-commit
* Apply updated pre-commit
* manual fixes
* manual fix RUF017

Signed-off-by: nstarman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk approved these changes

@neutrinoceros neutrinoceros neutrinoceros approved these changes

@dhomeier dhomeier dhomeier approved these changes

@eerovaher eerovaher eerovaher approved these changes

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

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

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

Assignees

No one assigned

Labels

config coordinates dev-automation io.ascii io.fits logging no-changelog-entry-needed skip-changelog-checks Tells bot to skip changlog checks table testing timeseries units utils.masked utils

Projects

None yet

Milestone

v6.1.0

Development

Successfully merging this pull request may close these issues.

6 participants

@nstarman @pllim @dhomeier @mhvk @neutrinoceros @eerovaher