Skip to content

add pyupgrade to pre-commit#13396

Merged
pllim merged 3 commits intoastropy:mainfrom
nstarman:pre-commit-pyupgrade
Jul 1, 2022
Merged

add pyupgrade to pre-commit#13396
pllim merged 3 commits intoastropy:mainfrom
nstarman:pre-commit-pyupgrade

Conversation

@nstarman
Copy link
Copy Markdown
Member

@nstarman nstarman commented Jun 23, 2022

Pyupgrade upgrades syntax for newer versions of the language. Some features, like f-strings, are faster. Other features improve code style. All upgrades ensure code contributions meet the best practices of our minimum python version.
Also, when we bump a python version we can run pre-commit run —all-files to upgrade all existing code.

I also cleaned up the YAML with a YAML cleaner.

Signed-off-by: Nathaniel Starkman (@nstarman) nstarkman@protonmail.com

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • 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 "When to rebase and squash commits".
  • 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.
  • 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 astropy-bot check might be missing; 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.

@nstarman nstarman added this to the v5.2 milestone Jun 23, 2022
@github-actions
Copy link
Copy Markdown
Contributor

👋 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?

@mhvk
Copy link
Copy Markdown
Contributor

mhvk commented Jun 23, 2022

I found the changes elsewhere all pretty clear improvements, so would be happy to add this.

@nstarman
Copy link
Copy Markdown
Member Author

nstarman commented Jun 23, 2022

I'll rebase this PR when all the other ones are in. Hopefully it will pass / require very little further work.

@nstarman nstarman mentioned this pull request Jun 23, 2022
10 tasks
@WilliamJamieson
Copy link
Copy Markdown
Contributor

Do we want to exclude some directories from pyupgrade like astropy/extern?

@nstarman nstarman mentioned this pull request Jun 24, 2022
10 tasks
@nstarman
Copy link
Copy Markdown
Member Author

Do we want to exclude some directories from pyupgrade like astropy/extern?

Yes, definitely a good idea.

Also, I'll ignore all the files in units and coordinates that are generated by extern/ply.

@nstarman nstarman force-pushed the pre-commit-pyupgrade branch from 665a9b0 to c48cb73 Compare June 24, 2022 17:12
@nstarman nstarman force-pushed the pre-commit-pyupgrade branch 2 times, most recently from 8cb4005 to c62f306 Compare June 30, 2022 19:46
@nstarman
Copy link
Copy Markdown
Member Author

nstarman commented Jul 1, 2022

I need to rebase to capture the timeseries related merge, but GTG otherwise

@nstarman nstarman marked this pull request as ready for review July 1, 2022 00:40
nstarman added 3 commits July 1, 2022 11:55
and clean up the YAML with a YAML cleaner

Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
@nstarman nstarman force-pushed the pre-commit-pyupgrade branch from c62f306 to 9caaf11 Compare July 1, 2022 15:58
@nstarman
Copy link
Copy Markdown
Member Author

nstarman commented Jul 1, 2022

Rebased and ready for review. astropy/coordinates has a few changes. I don't think I ran pyupgrade with —py38-plus, when I pushed #13379.

Copy link
Copy Markdown
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

I don't grok the pre-commit stuff so maybe @WilliamJamieson can look at that one. But the other changes LGTM. Thanks!

Copy link
Copy Markdown
Contributor

@WilliamJamieson WilliamJamieson left a comment

Choose a reason for hiding this comment

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

The pre-commit looks fine to me, not sure about the rest of the changes.

@pllim pllim merged commit 059adf9 into astropy:main Jul 1, 2022
@pllim
Copy link
Copy Markdown
Member

pllim commented Jul 1, 2022

Thanks!

@nstarman nstarman deleted the pre-commit-pyupgrade branch July 1, 2022 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants