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

Add test case showing issue with isort for missing transient deps. #15002

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kaos
Copy link
Member

@kaos kaos commented Apr 5, 2022

It seems there are cases even in fmt that requires transient deps to be present, not only for check.
See related comment: #14186 (comment)

I've run across this a couple of times now at work, where I have a pre-commit hook that runs on a subset of files (exactly the subset of changed files only), and fails, where as when I run by hand I usually use globs like a hammer and includes a whole lot more, which exposes this issue, due to isort not being stable for the two ways it is being invoked.

@kaos
Copy link
Member Author

kaos commented Apr 5, 2022

I found this isort option, which feels relevant, but I've not yet figured out how to use it to solve this issue (which I feel it ought to do): https://pycqa.github.io/isort/docs/configuration/options.html#src-paths

@thejcannon
Copy link
Member

thejcannon commented Apr 6, 2022

I think the correct route to take would be to use pants' knowledge of the python sources to seed its config.

@kaos
Copy link
Member Author

kaos commented Apr 7, 2022

I think the correct route to take would be to use pants' knowledge of the python sources to seed its config.

That would be great. I've not found out what config makes this work, though, besides including the first party sources being imported from the file being formatted.

I've played around with the src_paths config, and though I can see isort picking up a bunch of source files for that option (using ./pants fmt --only=isort --isort-args=--show-config), it still doesn't produce consistent output as I want it to.

@benjyw
Copy link
Contributor

benjyw commented May 31, 2022

See my comment in #15069, but isort's statement in https://pycqa.github.io/isort/docs/configuration/options.html#src-paths that "modules within src paths have their imports automatically categorized as first_party" is not entirely true - there is a caveat for namespace packages... I propose a solution there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants