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

First-party library (incorrectly?) detected as third-party depending on working directory #1497

Closed
bersbersbers opened this issue Sep 25, 2020 · 14 comments
Labels
question Further information is requested

Comments

@bersbersbers
Copy link

I cannot post my a fully-contained working environment here, but this is my example code, and maybe it's enough to reproduce or at least understand the issue.

This is /home/bers/myproj/myproj/tool/bug.py:

import os

import isort

import myproj


def _test():
    code = isort.code(open(__file__).read(), import_heading_thirdparty="3rd")
    print(code[: code.find("\n\n\ndef")] + "\n")


print("correct:")
os.chdir("/home/bers/myproj/myproj")
_test()

print("incorrect:")
os.chdir("/home/bers/myproj/myproj/tool")
_test()

What this outputs is this:

correct:
import os

# 3rd
import isort

import myproj

incorrect:
import os

# 3rd
import myproj
import isort

So you see that isort detects myproj as a 3rd-party library depending on the working directory - is this intended?

It may be relevant that I usually do pip install -e . from /home/bers/myproj when setting up environments - this is what makes import myproj possible from within /home/bers/myproj/myproj/tool/bug.py.

@char101
Copy link

char101 commented Sep 26, 2020

Simpler test case:

  1. install a third party library pip install path.py
  2. create a first party library config.py, just an empty file
  3. test script

main.py

from path import Path
from config import CONSTANT

When isort is run in the same directory as main.py:

from path import Path

from config import CONSTANT

When isort is not run in the same directory as main.py:

from config import CONSTANT
from path import Path

@char101
Copy link

char101 commented Sep 26, 2020

Source of the problem: https://github.com/PyCQA/isort/blob/develop/isort/settings.py#L388

directory is set to os.getcwd() which then overrides path_root.

@timothycrosley
Copy link
Member

timothycrosley commented Sep 27, 2020

Like most command line tools, isort uses the current working directory as a fallback if more explicit information isn't provided. See: https://pycqa.github.io/isort/docs/configuration/config_files/. Creating a config file at the root of your project (where you want lookup to occur) or setting your src paths manually should resolve this issue for you.

Let me know if you run into any issues with this,

Thanks!

~Timothy

@timothycrosley timothycrosley added the question Further information is requested label Sep 27, 2020
@char101
Copy link

char101 commented Sep 27, 2020

I don't think the definition of a first party library depends on where isort is run but on where the source file is located. I haven't upgraded isort for several months but the old version does not have this problem.

For information, with my test case, isort detects config as first party when run in the same directory as main.py, and as third party when run from other directory.

@timothycrosley
Copy link
Member

timothycrosley commented Sep 27, 2020

@char101 please take a look at the upgrade guide here: https://pycqa.github.io/isort/docs/upgrade_guides/5.0.0/#module-placement-changes-known_third_party-known_first_party-default_section-etc and associated issue here:#1147. The old approach had much bigger problems, but can still be used if desired as mentioned in the upgrade guide. That said, it is easy to tell isort exactly what you want from source paths etc. The only way to do what the old approach did, is to rely on a lot of very very fragile magic like what is loaded in the venv.

@char101
Copy link

char101 commented Sep 27, 2020

https://pycqa.github.io/isort/docs/upgrade_guides/5.0.0/#module-placement-changes-known_third_party-known_first_party-default_section-etc links to #1147, of which the 6th point is

  • FIRSTPARTY: import code exists in same directory as any files isort is told to sort.

So shouldn't config be categorized as first party since config.py is in the same directory as main.py?

@timothycrosley
Copy link
Member

timothycrosley commented Sep 27, 2020

Yes, and your example shows that it is. However, think about it, if you run it in a directory other than that, isort has no explicit way to know then your project starts or ends. Just because you have a config.py at the same level, doesn't mean that it isn't nested 10 levels in your source tree and so using it to categorize would be inappropriate. src-paths (https://pycqa.github.io/isort/docs/configuration/options/#src-paths) or an isort compatible config at the root of your project are how you tell isort where your project sources start / are located.

@char101
Copy link

char101 commented Sep 27, 2020

Ok I can understand that. Still, using the currect directory as the project root has this unintended consequence:

Let's assume this project structure

/dir
/dir/config.py
/dir/a/main.py

main.py

import config
import path

/dir$ isort a/main.py

Output:

import path

import config

config is considered first party just because it exists in the directory where isort is run, although it is impossible for main.py to import it unless /dir is added to PYTHONPATH.

IMO unless src-paths is given, it is better to take the directory of the given file as the project root rather than using the current directory.

@timothycrosley
Copy link
Member

timothycrosley commented Sep 27, 2020

I don't think this is a productive line of reasoning. Sure, there are reasons why defaulting to the path could give you the right answer, there are also reasons why defaulting to the current working directory could give you the right answer. After all, it is very common usage to have isort as a CI/CD tool that runs in the root of a project with many nested modules. It is also very common for users to run it in the root of the project. For both of these cases current working directory gives you the right answer, and path based does not. It is easy to come up with examples for both behaviours. However, if isort as a project switches back and forth for each example it will just make both use cases unpredictable. I think a better line of discussion: what things are there in your project structure that isort can use to know when it reached the top your project so it doesn't need to guess? And, is there a reason you cant put an isort configuration at the root of your project, again, so isort doesn't need to guess?

@char101
Copy link

char101 commented Sep 28, 2020

The thing is python are often used to write scripts. In this case the user doesn't think it as a project. First one creates main.py, then put helper functions in utils.py, then put constants in config.py. So there is no project, it is just a bunch of modules. And the user might not even run isort manually, it could be an editor plugin that runs when the file is saved.

@timothycrosley
Copy link
Member

timothycrosley commented Sep 28, 2020

@char101 if you have a pile of scripts, in a larger application context, there is no reason not to take one second to touch an .isort.cfg file where you wont those scripts to start. Also, in that context, you should be fine running isort in that same directory. If you are running isort via an editor it is just as likely it is configured to CD into the root of your project before running isort, in which case CWD behaviour would be helpful. Again, this line of discussion is just going to be going in circles, because it's a question of what is the best default, when both or equally valid - and it is very unlikely that it will be changed for the duration of the 5.0.0 release. I'm sorry the default isn't your preferred one, but I'd rather focus on real life use cases that you are running into, and how the project can offer solutions to them.

@char101
Copy link

char101 commented Sep 28, 2020

I don't have any problem with it currently. Maybe because it used to just works, so it feels like a bit of a regression. Ok so creating an empty .isort.cfg works, it should make it a lot easier.

The way I am using isort currently is in a MessagePack RPC server (with mprpc) along with yapf , pylint (pylint is slow when run via cli), pycodestyle, and pyflakes. My editor (vim) then calls the rpc service when saving a file to format and lint it. So far I just pass the file directory to the directory option when calling isort.api.sort_code_string.

@bersbersbers
Copy link
Author

bersbersbers commented Sep 28, 2020

Adding

[tool.isort]
src_paths = ["myproj"]

to my previously empty /home/bers/myproj/pyproject.toml seems to have fixed the issue for me, at least when running isort from within VS Code. (I can still reproduce the issue manually, but I don't care much about that.)

@timothycrosley
Copy link
Member

I'm going to close this, as the original opening question is now resolved. @char101 and @bersbersbers please feel empowered to open additional issues if I missed anything.

Thanks!

~Timothy

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

No branches or pull requests

3 participants