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

isort with multiple config files does not work correctly #17327

Closed
AbdealiLoKo opened this issue Oct 22, 2022 · 5 comments
Closed

isort with multiple config files does not work correctly #17327

AbdealiLoKo opened this issue Oct 22, 2022 · 5 comments
Labels
backend: Python Python backend-related issues bug

Comments

@AbdealiLoKo
Copy link

AbdealiLoKo commented Oct 22, 2022

Describe the bug
I have a project like:

libs
    lib1
        pyproject.toml
        lib1
            ... files in the lib
        lib1_test
            ... tests for the lib
    lib2
        ... similar to lib1
    ... more libs

And I am using pyproject.toml for the isort configs:

[tool.isort]
profile = "pycharm"
skip_glob = ["venv*"]
line_length = 110
known_first_party = ["lib1", "lib1_test"]
combine_as_imports = true
force_grid_wrap = 0
include_trailing_comma = true
order_by_type = false
case_sensitive = true

And the pyproject.toml for every lib changes the known_first_party = ["lib1", "lib1_test"] to treat only the library as a first party import.

When I run: ./pants fmt :: - it was using the wrong first party imports
On @benjyw 's suggestio - tried: ./pants fmt libs/lib1/:: And it seemed to work

After a lot of digging, I realized that isort finds the config file based on the 1st file provided to it
https://pycqa.github.io/isort/docs/configuration/config_files.html
And order of files that pants is sending into isort is mosst likely the culprit here.

Pants version: 2.13.0
OS: Windows 10 + Ubuntu 18.04 LTS

Additional info
Slack conversation: link
Another issue related to isort I checked: #15069
The batch ID concept in this issue may have been a workaround here: #14941

@thejcannon
Copy link
Member

In this case I believe the solution would be to partition the inputs (files) based on config such that no two files passed to isort would disagree on config.
This is how we run scalafmt:

async def partition_scalafmt(

@thejcannon thejcannon added the backend: Python Python backend-related issues label Oct 24, 2022
@AbdealiLoKo
Copy link
Author

On discussion with @Eric-Arellano This issue does not seem to be present with isort4
In isort4 - the config files were not based on the "first input provided to isort" (althought I don't have any lear documentation to referencec here)

I tried:

$ venv/bin/pip install -U 'isort<5'
Successfully installed isort-4.3.21

$ venv/bin/isort lib2 lib1 <---- Formatted something
Fixing lib2/lib2/__init__.py
Fixing lib1/lib1/__init__.py

$ venv/bin/isort lib1 lib2 <----- FLIP: nothing to fix.

$ venv/bin/pip install -U isort 
Successfully installed isort-5.10.1

$ venv/bin/isort lib2 lib1 <---- Formatted something
Fixing lib2/lib2/__init__.py
Fixing lib1/lib1/__init__.py

$ venv/bin/isort lib1 lib2 <----- FLIP: Still has something to fix
Fixing lib1/lib1/__init__.py
Fixing lib2/lib2/__init__.py

@Eric-Arellano
Copy link
Contributor

From Slack:

Looks like there is a argument --resolve-all-configs which follows the older behavior of getting a config file for every file provided as input

@AbdealiJK can this be closed, then? Is that working?

@AbdealiLoKo
Copy link
Author

Yep - closing.
I am still seeing some weird behavior. Specifically in this structure:

my_api
pyproject.toml
libs/
    lib1
        lib1
        pyproject.toml

But when I run: venv/bin/pants fmt lib/lib1/lib1 it considers my_api as a 3rd party import
And running: venv/bin/pants fmt :: it considers my_api as a 1st party import

But I assume this is because isort --resolve-all-configs merges the root pyproject.toml with the libs/lib1/pyproject.toml file.

So, I think that is because of the sandbox not having all the files - maybe same as: #15069

@benjyw
Copy link
Sponsor Contributor

benjyw commented Oct 26, 2022

Yeah, that last issue sounds like #15069, and the workaround is to explicitly list your 1stparty packages in isort config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues bug
Projects
None yet
Development

No branches or pull requests

4 participants