https://pantsbuild.org/ logo
Join Slack
Powered by
I was able to setup my project based on the guidan...
# general
l
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
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
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
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
Ah - I see. makes sense
b
(I'll also relay this on the ticket if you post the link here)
✅ 1
l
h
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
@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
Correct, it won’t fix that
h
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
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
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
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
Oh, this is outside of Pants!
iiiiiiiiiinteresting
l
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
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
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
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
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
Good find! Should we be setting this by default for isort >=5, or document this at least?
h
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
Open in Slack
PreviousNext
Powered by