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.

Sign up for GitHub

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

Jump to bottom

Do not require built-in sections to be present in section-order #7868

Closed
levrik opened this issue Oct 9, 2023 · 3 comments · Fixed by #10149
Closed

Do not require built-in sections to be present in section-order #7868

levrik opened this issue Oct 9, 2023 · 3 comments · Fixed by #10149
Labels
isort Related to import sorting needs-decision Awaiting a decision from a maintainer

Comments

@levrik
Copy link

levrik commented Oct 9, 2023 •
edited
Loading

I'm currently in the process of migrating from flake8 and isort to Ruff.

Something I've noticed is that when leaving out a built-in section from section-order, a warning appears and these sections are appended to the end of section-order. With isort imports that correspond to a built-in section that is missing from section-order, are assigned to the third-party section instead. I used this before to combine standard-library and third-party sections into a single one as I don't see much value in separating these.

As a workaround I've added all packages from https://github.com/astral-sh/ruff/blob/main/crates/ruff_python_stdlib/src/sys.rs to known-third-party but it's rather ugly.

It would be great if the behavior could match isort or could be configured to match it somehow.
@charliermarsh charliermarsh added isort Related to import sorting needs-decision Awaiting a decision from a maintainer labels Oct 9, 2023
@mmerickel
Copy link
Contributor

mmerickel commented Oct 30, 2023 •
edited
Loading

Ran into this as well, isort supports a default_section for this purpose if you want to be explicit:

sections = "FUTURE,THIRDPARTY,FIRSTPARTY,LOCALFOLDER"
default_section = "THIRDPARTY"

The end goal is that stdlib and third party are combined, as it's common in the libraries that I work on that we are using bw-compat shims etc and it's irritating to do treat things like importlib_metadata differently from importlib.metadata. Yes you could define them explicitly but ... the extended philosophy is that the stdlib and third party should be the same from the perspective of a user - that's why we put all this effort into moving stuff into pypi instead of in the stdlib, they aren't that different!

@mmerickel
Copy link
Contributor

Looks like something along similar lines was just merged #8657. It would've been possible to implement no_sections from that PR with the equivalent proposal above:

sections = "THIRDPARTY"
default_section = "THIRDPARTY"

@mmerickel mmerickel mentioned this issue Feb 28, 2024
Merged
@mmerickel
Copy link
Contributor

I've implemented default-section in #10149.

charliermarsh pushed a commit that referenced this issue Mar 1, 2024
@mmerickel
Implement isort's default-section setting (#10149)
c9931a5 
## Summary

This fixes #7868.

Support isort's `default-section` feature which allows any imports that
match sections that are not in `section-order` to be mapped to a
specifically named section.


https://pycqa.github.io/isort/docs/configuration/options.html#default-section

This has a few implications:

- It is no longer required that all known sections are defined in
`section-order`.
- This is technically a bw-incompat change because currently if folks
define custom groups, and do not define a `section-order`, the code used
to add all known sections to `section-order` while emitting warnings.
**However, when this happened, users would be seeing warnings so I do
not think it should count as a bw-incompat change.**

## Test Plan

- Added a new test.
- Did not break any existing tests.

Finally, I ran the following config against Pyramid's complex codebase
that was previously using isort and this change worked there.

### pyramid's previous isort config


https://github.com/Pylons/pyramid/blob/5f7e286b0629b0a5f1225fe51172cba77eb8fda1/pyproject.toml#L22-L37

```toml
[tool.isort]
profile = "black"
multi_line_output = 3
src_paths = ["src", "tests"]
skip_glob = ["docs/*"]
include_trailing_comma = true
force_grid_wrap = false
combine_as_imports = true
line_length = 79
force_sort_within_sections = true
no_lines_before = "THIRDPARTY"
sections = "FUTURE,THIRDPARTY,FIRSTPARTY,LOCALFOLDER"
default_section = "THIRDPARTY"
known_first_party = "pyramid"
```

### tested with ruff isort config

```toml
[tool.ruff.lint.isort]
case-sensitive = true
combine-as-imports = true
force-sort-within-sections = true
section-order = [
    "future",
    "third-party",
    "first-party",
    "local-folder",
]
default-section = "third-party"
known-first-party = [
    "pyramid",
]
```
nkxxll pushed a commit to nkxxll/ruff that referenced this issue Mar 10, 2024
@mmerickel @nkxxll
Implement isort's default-section setting (astral-sh#10149)
14ceaec 
## Summary

This fixes astral-sh#7868.

Support isort's `default-section` feature which allows any imports that
match sections that are not in `section-order` to be mapped to a
specifically named section.


https://pycqa.github.io/isort/docs/configuration/options.html#default-section

This has a few implications:

- It is no longer required that all known sections are defined in
`section-order`.
- This is technically a bw-incompat change because currently if folks
define custom groups, and do not define a `section-order`, the code used
to add all known sections to `section-order` while emitting warnings.
**However, when this happened, users would be seeing warnings so I do
not think it should count as a bw-incompat change.**

## Test Plan

- Added a new test.
- Did not break any existing tests.

Finally, I ran the following config against Pyramid's complex codebase
that was previously using isort and this change worked there.

### pyramid's previous isort config


https://github.com/Pylons/pyramid/blob/5f7e286b0629b0a5f1225fe51172cba77eb8fda1/pyproject.toml#L22-L37

```toml
[tool.isort]
profile = "black"
multi_line_output = 3
src_paths = ["src", "tests"]
skip_glob = ["docs/*"]
include_trailing_comma = true
force_grid_wrap = false
combine_as_imports = true
line_length = 79
force_sort_within_sections = true
no_lines_before = "THIRDPARTY"
sections = "FUTURE,THIRDPARTY,FIRSTPARTY,LOCALFOLDER"
default_section = "THIRDPARTY"
known_first_party = "pyramid"
```

### tested with ruff isort config

```toml
[tool.ruff.lint.isort]
case-sensitive = true
combine-as-imports = true
force-sort-within-sections = true
section-order = [
    "future",
    "third-party",
    "first-party",
    "local-folder",
]
default-section = "third-party"
known-first-party = [
    "pyramid",
]
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
isort Related to import sorting needs-decision Awaiting a decision from a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement isort's default-section setting mmerickel/ruff
3 participants
@mmerickel @charliermarsh @levrik