ruff icon indicating copy to clipboard operation
ruff copied to clipboard

add type checking to isort section order

Open fraser-langton opened this issue 1 year ago • 2 comments

I am not sure if I just kind find the docs, because I expected this to be included in either I (isort) or TCH (flake8-type-checking).

Although no PEP recommends where TYPE_CHECKING import blocks should go I think it's pretty standard to put after all other imports

ie I would expect the default value for [tool.ruff.lint.isort.section-order] to be ["future", "standard-library", "first-party", "local-folder", "third-party", "type-checking"]

(sorry if this is already included, I cannot find it anywhere)

fraser-langton avatar Oct 29 '24 04:10 fraser-langton

Hi @fraser-langton

I think it's pretty standard to put after all other imports

Could you share some reference suggesting that putting TYPE_CHECKING last is the standard? I did a quick github code search and the samples I found all have the typing imports together with the other standard library imports.

What sorting do you suggest when there are other typing imports besides TYPE_CHECKING, e.g. typing?

MichaReiser avatar Oct 29 '24 06:10 MichaReiser

I think maybe I wasn't as clear as I should have been, is mean the if TYPE_CHECKING: block.

For example TCH --fix --unsafe-fixes put them last.

Maybe the isort rules don't have a default, but it would be good to at least configure [tool.ruff.lint.isort.section-order] = ["future", "standard-library", "first-party", "local-folder", "third-party", "type-checking"] - maybe if-type-checking is a clearer section name

fraser-langton avatar Oct 29 '24 22:10 fraser-langton

For example TCH --fix --unsafe-fixes put them last.

I think this only happens when the rules autofix would introduce a TYPE_CHECKING block like TCH001-003.

Is the feature request to allow configuring the position of this insertion of TYPE_CHECKING block?

dhruvmanila avatar Oct 30 '24 03:10 dhruvmanila

@dhruvmanila not inserting it, configure how it should be sorted in [tool.ruff.lint.isort.section-order] - ie so it must always be after all imports if you want

fraser-langton avatar Oct 30 '24 03:10 fraser-langton

I don't think this should be part of isort. isort is only concerned with sorting imports and what I understand is that you want to configure the position of the if TYPE_CHECKING block.

I'm open to iterating on the TCH fix to placing the TYPE_CHECKING block directly after all imports, if it is safe to do so.

MichaReiser avatar Oct 30 '24 06:10 MichaReiser

There is a particular style I use for typing imports I would like to enforce that has always stopped me from enabling isort and it's somewhat related to this request. I generally use the following style:

from decimal import Decimal

from third_party import Foo

from my_module import Bar


from typing import Any
from typing import TYPE_CHECKING
if TYPE_CHECKING:
    from collections.abc import Sequence
    from typing_extensions import Self

    from typing_only_third_party import FooType

    from my_typing_only import BarType

i.e. the typing imports are all grouped together with the first if TYPE_CHECKING block and separated by two spaces from the other sections to clearly show which part of the import block is relevant for runtime and which part is only used for type hints.

So adding the ability to define some more complex layouts like this one would be appreciated, but it would likely involve having to switch to a template string for defining the layout of the sections and some smart logic for collapsing white space between missing sections and allowing the sections within an if TYPE_CHECKING: block to be different. Note how in my example typing and typing_extensions are in their own section in runtime scope but are folded into standard-library within typing only scope.

Daverball avatar Oct 30 '24 07:10 Daverball

It was unexpected to me, that, e.g., the following order is not flagged by isort (or any other rule like TCH or E402):

import os
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from my_package import my_function

import pydantic

import my_package

kdebrab avatar Oct 31 '24 07:10 kdebrab

Looking at this from a different angle: Could this be related to E402 instead ? "Module level import not at top of file" could trigger for imports found after top-level if TYPE_CHECKING blocks.

Avasam avatar Nov 03 '24 17:11 Avasam

@MichaReiser

I don't think this should be part of isort. isort is only concerned with sorting imports and what I understand is that you want to configure the position of the if TYPE_CHECKING block.

I agree with this, probably not in isort as you really only want it to be last (ie you wouldn't configure something like ["future", "standard-library", "first-party", "type-checking", "local-folder", "third-party"]

I'm open to iterating on the TCH fix to placing the TYPE_CHECKING block directly after all imports, if it is safe to do so.

TCH does make a lot of sense to me, as it already inserts a TYPE_CHECKING block after all imports.

fraser-langton avatar Nov 06 '24 22:11 fraser-langton

@MichaReiser have you had any other thoughts regarding this?

fraser-langton avatar Dec 12 '24 03:12 fraser-langton

Yeah, I would also like TYPE_CHECKING to be at the end.

Sxderp avatar Jan 27 '25 15:01 Sxderp