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

STY: migrate formatting from black to ruff-format #4868

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

neutrinoceros
Copy link
Member

PR Summary

Closes #4748
The formatting commit is "semi-automated" because ruff was, in some places, somewhat akwardly moving around some strings formatted with %, but I found manually changing them to fstrings helped.

@neutrinoceros neutrinoceros added infrastructure Related to CI, versioning, websites, organizational issues, etc code style Related to linting tools labels Apr 3, 2024
@neutrinoceros
Copy link
Member Author

just a note on performance since it's the main selling point for ruff: pre-commit.ci run took 7.5s instead of the usual ~26s.

@neutrinoceros
Copy link
Member Author

@yt-fido test this please

1 similar comment
@neutrinoceros
Copy link
Member Author

@yt-fido test this please

@neutrinoceros neutrinoceros force-pushed the sty/black2ruff branch 2 times, most recently from bc7f15b to 4241c11 Compare April 3, 2024 07:26
@neutrinoceros neutrinoceros marked this pull request as ready for review April 3, 2024 14:25
@neutrinoceros
Copy link
Member Author

@matthewturk @cphyc @chrishavlin anyone up for reviewing this ?

@@ -47,3 +47,6 @@ ec8bb45ea1603f3862041fa9e8ec274afd9bbbfd

# auto upgrade typing idioms from Python 3.8 to 3.9
4cfd370a8445abd4620e3853c2c047ee3d649fd7

# migration: black -> ruff-format
61ca693b2c58eefc0ca5768ee4c3a8512b47504a
Copy link
Member

Choose a reason for hiding this comment

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

I think this isn't the right commit. It should be

Suggested change
61ca693b2c58eefc0ca5768ee4c3a8512b47504a
ce25254257185d53e7b3cacf39c48c35424cb753

should't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. Thanks for checking !

hooks:
- id: black-jupyter

- repo: https://github.com/adamchainz/blacken-docs
Copy link
Member

Choose a reason for hiding this comment

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

Sorry if I missed this information - but is ruff also running on docs? I couldn't find the confirmation online. The closest I got was astral-sh/ruff#8237, but then I don't see where ruff is configured to run on the doc?

Copy link
Member Author

Choose a reason for hiding this comment

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

hum. I may have tricked myself into believing that it was supported when I misunderstood something and didn't link the source in #4748 (comment)

I think the source of my confusion was astropy/astropy#16132, which doesn't have anything to do with embedded python blocks.
I'll revert this and keep blacken-docs around for a little longer, but I'll also link the ruff issue. Thanks !

@neutrinoceros neutrinoceros force-pushed the sty/black2ruff branch 2 times, most recently from f685f94 to d25b38b Compare April 19, 2024 09:37
pyproject.toml Outdated
| yt/frontends/stream/sample_data
)/
| yt/visualization/_colormap_data.py
'''
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove the black-specific configuration from pyproject.toml since it is still in use by blacken-docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I (mostly) reverted it, but I took the opportunity to clean up unnecessary parameters.

@neutrinoceros
Copy link
Member Author

I'm just going to merge now before pre-commit.ci auto PR create a merge conflict with this one. Thanks !

@neutrinoceros neutrinoceros merged commit cd4f1d6 into yt-project:main Apr 29, 2024
13 checks passed
@neutrinoceros neutrinoceros deleted the sty/black2ruff branch April 29, 2024 13:33
@neutrinoceros neutrinoceros added this to the 4.4.0 milestone Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code style Related to linting tools infrastructure Related to CI, versioning, websites, organizational issues, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

STY: switching from black to ruff-format ?
2 participants