isort icon indicating copy to clipboard operation
isort copied to clipboard

THIRDPARTY module detected as FIRSTPARTY when import happens within subdir of same name

Open jherland opened this issue 1 year ago • 1 comments

This is certainly related to #1696, but I'm not sure that it's a duplicate, as I think the relevant directory layout is slightly different.

Consider the minimal example project at https://github.com/jherland/fawltydeps-issue419-test [^1]: Inside app/fastapi/main.py, we import fastapi. This is a third-party import: with a correctly configured venv, Python will resolve the fastapi import to the fastapi package installed in that venv. Later, we import same_dir_module, which is a straigh-forward first-party import. This is the output I get when running python app/fastapi/main.py:

fastapi found at /home/jherland/.cache/pypoetry/virtualenvs/app-WtNxBkJt-py3.11/lib/python3.11/site-packages/fastapi/__init__.py
same_dir_module found at /home/jherland/code/fd-test-419/app/fastapi/same_dir_module.py
sys.path is ['/home/jherland/code/fd-test-419/app/fastapi', ..., '/home/jherland/code/fd-test-419']

However, when I attempt to classify these imports, isort.place_module() claims that both imports are FIRSTPARTY imports:

#!/usr/bin/env python3

import isort

config = isort.Config(src_paths=("app/fastapi", "."))

print(isort.place_module_with_reason("fastapi", config=config))
print(isort.place_module_with_reason("same_dir_module", config=config))

produces this output:

('FIRSTPARTY', 'Found in one of the configured src_paths: /home/jherland/code/fd-test-419/app/fastapi.')
('FIRSTPARTY', 'Found in one of the configured src_paths: /home/jherland/code/fd-test-419/app/fastapi.')

Trying to examine what happens inside isort, I find these lines to be relevant (from _src_path() in isort/place.py):

  1. AFAICS, these lines cause the _is_module(module_path) test below to trigger whenever a 3rd-party import happens from within a file located in a directory with the same name as the import:
            if not prefix and not module_path.is_dir() and src_path.name == root_module_name:
                module_path = src_path.resolve()
    
  2. Also, the last or-clause of the if condition will trigger in the same circumstance.
                or _src_path_is_module(src_path, root_module_name)
    

I am not sure if I'm just "holding it wrong" here? At least, I'm unable to find a config for isort that will agree with Python's import resolver here, i.e. classify fastapi as THIRDPARTY and same_dir_module as FIRSTPARTY.

[^1]: This report is based on https://github.com/tweag/FawltyDeps/issues/419 and @jharrisonSV's original example at https://github.com/jharrisonSV/fawltydeps-test

jherland avatar Mar 14 '24 12:03 jherland

The following patch works for me as a workaround. I do not see which cases the commented-out lines are supposed to cover.

In practice, there is a confusion about the semantics of src_paths. A src_path like somewhere/foo containing somewhere/foo/bar.py will bring both foo and bar as importable names. I would argue that only one of these should be imported.

diff --git a/isort/place.py b/isort/place.py
index 52ddc122..7f6f00e9 100644
--- a/isort/place.py
+++ b/isort/place.py
@@ -76,8 +76,9 @@ def _src_path(
 
     for src_path in src_paths:
         module_path = (src_path / root_module_name).resolve()
-        if not prefix and not module_path.is_dir() and src_path.name == root_module_name:
-            module_path = src_path.resolve()
+        print(module_path, root_module_name)
+        #if not prefix and not module_path.is_dir() and src_path.name == root_module_name:
+        #    module_path = src_path.resolve()
         if nested_module and (
             namespace in config.namespace_packages
             or (
@@ -89,7 +90,7 @@ def _src_path(
         if (
             _is_module(module_path)
             or _is_package(module_path)
-            or _src_path_is_module(src_path, root_module_name)
+            #or _src_path_is_module(src_path, root_module_name)
         ):
             return (sections.FIRSTPARTY, f"Found in one of the configured src_paths: {src_path}.")
 

layus avatar Feb 17 '25 12:02 layus