https://pantsbuild.org/ logo
#general
Title
# general
l

lively-dusk-46231

10/22/2022, 1:55 PM
I was able to setup my project based on the guidance in the above thread. I then ran
./pants fmt ::
And it successfully ran
black
and
isort
(and I saw it doing it on multiple cores) which was pretty neat. I then re-ran the same command and it ran super fast (10sec). I guessed that pants realized I had not modified these files, and so used some caching to Now, I realized that in one of my
libs/
- the isort config for
known_first_party
was not correct. So, I fixed that in my
pyproject.toml
And reran:
./pants fmt ::
ISSUE: It says nothing needs to be changed. When I format it using:
venv/bin/isort libs/mylib/
it uses the correct configs. When I again run:
./pants fmt ::
- it reformats it back using the older configs
Is there any specific config I need to write in BUILD to tell pants that my isort/black configs are in pyproject.toml ? (I read in documentation that manually invalidating cache in pants v2 isnt recommended)
Here I see that pants should have auto discovered my config file (I don't run
isort
with a custom
--config
)
Hmm, even if I try to add a
[isort].config
Which one do I add ? I have 8 different pyproject.toml files each of them have a different
known_first_party
I normally use it as:
libs/abc
will consider only
abc
as first party.
libs/xyz
should be considered a 3rd party import
h

happy-kitchen-89482

10/22/2022, 4:10 PM
First of all- youโ€™re correct about the caching, that is a big part of the design and purpose of pants! ๐Ÿ™‚
What happens if you run
./pants fmt libs/mylib/::
vs
./pants fmt ::
?
With
./pants fmt libs/mylib/::
is it picking up the right config?
l

lively-dusk-46231

10/22/2022, 4:55 PM
Hmmm, yeah
I've been digging into this for the past few hours. It looks like when pants is running isort - it runs on a batch of files And so probably does:
isort libs/abc/abc.py libs/xyz/xyz.py
According to isort docs :
[The config file taken is] ... relative to the first path passed in if multiple paths are passed in
So, if files from multiple libs are passed into isort - it will cause issues. i.e. Sorting depends on the ordering of the files sent to isort. Which is scary This seems to be related but not same as issue#15069 And maybe the BatchID concept described in issue#14941 would solve this?
b

bitter-ability-32190

10/22/2022, 5:07 PM
Ouch!
isort
continues to surprise me with it's quirks.
Can you file a GitHub issue? issue#15069 is related, yes, but different enough that I think we can fix this particular issue standalone.
issue#14941 is an interesting find, but that is more of a performance (and maybe a slight correctness on behalf of user pytest plugin) thing. In this case we really ought to be configuring and running
isort
for maximum correctness and since we know (or at least think we know) what`isort` is gonna do.
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`: https://github.com/pantsbuild/pants/blob/97c4ede10978fe20750ddc2642149d8b8a841126/src/python/pants/backend/scala/lint/scalafmt/rules.py#L125
๐Ÿ‘† 1
l

lively-dusk-46231

10/22/2022, 5:12 PM
Ah - I see. makes sense
b

bitter-ability-32190

10/22/2022, 5:13 PM
(I'll also relay this on the ticket if you post the link here)
โœ… 1
l

lively-dusk-46231

10/22/2022, 5:15 PM
h

happy-kitchen-89482

10/23/2022, 1:15 PM
Thanks! I think that is exactly right, we need to take different isort configs in different parts of the repo into account when batching
b

bitter-ability-32190

10/23/2022, 1:19 PM
@happy-kitchen-89482 *partitioning (trying to use the right terminology)
Also I don't think this will fix https://github.com/pantsbuild/pants/issues/15069 although it might make the occurrence rate slightly smaller
h

happy-kitchen-89482

10/23/2022, 4:55 PM
Correct, it wonโ€™t fix that
h

hundreds-father-404

10/24/2022, 3:12 PM
Hm did this change from isort 4? In isort 4, it supports multiple config files. You only need to not tell isort what to use, and to do autodiscovery. Pants handles that correctly, and the Pants project used to have multiple config files
@lively-dusk-46231 what was your behavior without using Pants? We should be only lightly wrapping isort, such that it's original behavior is preserved
l

lively-dusk-46231

10/24/2022, 3:17 PM
In my earlier setup I didn't have a monorepo and so I had a single isort config in every repo of mine. What would you like me to test ?
h

happy-kitchen-89482

10/24/2022, 3:38 PM
Oh, interesting
Yeah, maybe that is not the issue after all
Maybe this is all some variant of https://github.com/pantsbuild/pants/issues/15069 after all
l

lively-dusk-46231

10/24/2022, 3:47 PM
I don't think so @happy-kitchen-89482 When I run isort with 2 folders:
lib1
and
lib2
If I interchange the order of the folders I pass into isort - it keeps using different
known_first_party
from the first folder I mention in the command Example: Flipping the folders goes into an endless loop of fixing:
Copy code
$ venv/bin/isort lib2 lib1 <----
Fixing lib2/lib2/__init__.py
Fixing lib1/lib1/__init__.py

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

$ venv/bin/isort lib1 lib2 <----- FLIP: Fixing again
Fixing lib1/lib1/__init__.py
Fixing lib2/lib2/__init__.py
This is with
isort 5.10.1
h

happy-kitchen-89482

10/24/2022, 3:49 PM
Oh, this is outside of Pants!
iiiiiiiiiinteresting
l

lively-dusk-46231

10/24/2022, 3:49 PM
But this is expected behavior based on isort's documentation
https://pycqa.github.io/isort/docs/configuration/config_files.html
The config file search is done relative... to the first path passed in if multiple paths are passed in
Copy code
$ venv/bin/isort lib1 lib2 --show-config | grep '"known_first_party"'
"known_first_party": ["lib1"]

$ venv/bin/isort lib2 lib1 --show-config | grep '"known_first_party"'
"known_first_party": ["lib2"]
h

hundreds-father-404

10/24/2022, 4:17 PM
I'm pretty sure this changed from isort 4 then, which robustly supported multiple config files being used at the same time. You could switch back to isort 4 by changing
[isort].version
and
[isort].lockfile
, then running
./pants generate-lockfiles --resolve=isort
l

lively-dusk-46231

10/25/2022, 4:27 AM
So, I was unable to use
isort=4
in my project because it looks like there were breaking changes in the configs and so on and I was not too keen figuring out the correct configs for isort4 But I created a smaller example with multiple configs in
isort4
and agree that isort4 does not seem to have this issue
h

happy-kitchen-89482

10/25/2022, 3:38 PM
Ooooooof
It would be good to find any documentation of this issue in the isort changelog
โž• 1
was this a purposeful decision, or is it a bug there?
l

lively-dusk-46231

10/25/2022, 4:00 PM
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
โ— 1
๐Ÿ‘€ 1
๐Ÿ‘ 1
h

happy-kitchen-89482

10/26/2022, 3:37 PM
Good find! Should we be setting this by default for isort >=5, or document this at least?
h

hundreds-father-404

10/26/2022, 3:58 PM
maybe we should? I'm surprised isort doesn't have it on by default. Our config file support is very much designed to support multiple files, e.g. config discovery includes all the relevant ones
5 Views