PyCQA / isort

A Python utility / library to sort imports.
https://pycqa.github.io/isort/
MIT License
6.53k stars 583 forks source link

Searching for project_root #2293

Open JulienPalard opened 2 months ago

JulienPalard commented 2 months ago

I found strange that having a pyproject.toml with:

[tool.isort]
some = "config"

behaved differently than just having:

[tool.isort]

So I dug settings.py to try to understand the logic.

The _find_config function tries hard (and succeed) at finding the project root in my case, I typically have a .git/, it's found, nice.

The caller of _find_config have a nice project_root variable, initialized with the right value for me.

But a few lines later I see:

        if "directory" not in combined_config:
            combined_config["directory"] = (
                os.path.dirname(config_settings["source"])
                if config_settings.get("source", None)
                else os.getcwd()
            )

        path_root = Path(combined_config.get("directory", project_root)).resolve()

To simplify I see:

        if "directory" not in combined_config:
            combined_config["directory"] = something

        path_root = Path(combined_config.get("directory", project_root)).resolve()

So directory is always set, so the .get("directory" is superfluous, it can be simplified as:

        if "directory" not in combined_config:
            combined_config["directory"] = something

        path_root = Path(combined_config["directory"]).resolve()

But doing so the only place where project_root was used is lost. So currently, the project_root variable is initialized but never used.

Instead, if there's a config source (being the directory in which the configuration file is found) the config source is used, that explains why when I set a non empty [tool.isort] in my pyproject.toml, my project root is correctly determined.

So my question is: what is the expected behavior for project root finding?

I see two alternatives:

But I have not found a documentation about this, I found #1239 that is related though.