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

Retry section lookup for from+import statements, if defaulted #2168

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions isort/deprecated/finders.py
Expand Up @@ -377,12 +377,12 @@ def __init__(
)
self.finders: Tuple[BaseFinder, ...] = tuple(finders)

def find(self, module_name: str) -> Optional[str]:
def find(self, module_name: str) -> Tuple[Optional[str], str]:
for finder in self.finders:
try:
section = finder.find(module_name)
if section is not None:
return section
return section, ""
except Exception as exception:
# isort has to be able to keep trying to identify the correct
# import section even if one approach fails
Expand All @@ -391,4 +391,4 @@ def find(self, module_name: str) -> Optional[str]:
f"{finder.__class__.__name__} encountered an error ({exception}) while "
f"trying to identify the {module_name} module"
)
return None
return None, ""
12 changes: 9 additions & 3 deletions isort/parse.py
Expand Up @@ -155,7 +155,7 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte

finder = FindersManager(config=config).find
else:
finder = partial(place.module, config=config)
finder = partial(place.module_with_reason, config=config)

line_count = len(in_lines)

Expand Down Expand Up @@ -438,7 +438,13 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte

if type_of_import == "from":
import_from = just_imports.pop(0)
placed_module = finder(import_from)
placed_module, reason = finder(import_from)
if placed_module == config.default_section and reason.startswith("Default"):
# The imported name might be a submodule of `import_from`, in which case
# it might be specified in the config.
# See https://github.com/PyCQA/isort/issues/2167.
placed_module, reason = finder(".".join([import_from] + just_imports))

if config.verbose and not config.only_modified:
print(f"from-type place_module for {import_from} returned {placed_module}")

Expand Down Expand Up @@ -552,7 +558,7 @@ def file_contents(contents: str, config: Config = DEFAULT_CONFIG) -> ParsedConte
import_index -= len(
categorized_comments["above"]["straight"].get(module, [])
)
placed_module = finder(module)
placed_module, reason = finder(module)
if config.verbose and not config.only_modified:
print(f"else-type place_module for {module} returned {placed_module}")

Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_isort.py
Expand Up @@ -5671,3 +5671,14 @@ def test_reexport_not_last_line() -> None:
meme = "rickroll"
"""
assert isort.code(test_input, config=Config(sort_reexports=True)) == expd_output


def test_import_submodule_both_ways() -> None:
"""See https://github.com/PyCQA/isort/issues/2167."""
test_input = "import requests\n" "\n" "from subdir import fileA\n"
test_output = isort.code(code=test_input, known_first_party=["subdir.fileA"])
assert test_output == ("import requests\n" "\n" "from subdir import fileA\n")

test_input = "import requests\n" "\n" "import subdir.fileA\n"
test_output = isort.code(code=test_input, known_first_party=["subdir.fileA"])
assert test_output == ("import requests\n" "\n" "import subdir.fileA\n")