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

refactor(ruff): Organize imports w/ (I001, TID252) rules #3513

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

dangotbanned
Copy link
Member

@dangotbanned dangotbanned commented Aug 2, 2024

This PR defines and applies import rules.

Something like this had been on my radar for a while, adding this now seems to be the least disruptive time.

Autogenerated code already had I001 applied for import sorting:

def ruff_write_lint_format_str(
fp: Path, code: str | Iterable[str], /, *, encoding: str = "utf-8"
) -> None:
"""
Combined steps of writing, `ruff check`, `ruff format`.
Notes
-----
- `fp` is written to first, as the size before formatting will be the smallest
- Better utilizes `ruff` performance, rather than `python` str and io
- `code` is no longer bound to `list`
- Encoding set as default
- `I001/2` are `isort` rules, to sort imports.
"""
commands = (
["ruff", "check", fp, "--fix"],
["ruff", "check", fp, "--fix", "--select", "I001", "--select", "I002"],

In addition to the visual changes, this makes navigating via imports much easier.

Ruff links

Related Issues

Imports that can benefit from this are already flagged by `FA100`. This just added boilerplate where it wasn't needed
`altair` is especially in need of this, due to the number of modules with identical names at different levels.
@mattijn mattijn merged commit 6c4c785 into vega:main Aug 2, 2024
14 checks passed
@mattijn
Copy link
Contributor

mattijn commented Aug 2, 2024

Looks good, thanks.

@dangotbanned dangotbanned deleted the ruff-organize-imports branch August 2, 2024 17:52
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.

3 participants