Ruff is removing used imports since 0.0.173 version

[Czaki]

Since version 0.0.173 (up to 0.0.178 - newest when create this issue) the ruff is removing used imports

I have two examples:

https://github.com/4DNucleome/PartSeg/blob/fadc4f8ad9b552386de21bbc2a1ab3e0159f52e7/package/PartSegCore/analysis/io_utils.py#L10-L24

is converted to:

and remove the type used in type annotation:
https://github.com/4DNucleome/PartSeg/blob/fadc4f8ad9b552386de21bbc2a1ab3e0159f52e7/package/PartSegCore/mask/io_functions.py#L725

The ProjectInfoBase is a Protocol if this information is important. In the second case, such import could be hidden under if typing.TYPE_CHECKING: but I use in code other things from this module.

And even if it should be moved under the type checking clause, the autofix should not just remove it.

[charliermarsh]

Thanks! Will fix. Sorry about that.

[charliermarsh]

In both cases the cause is the same, which is that the import appears to be redefined-while-unused (hence the NoQA), so we then mark the import as unused.

I could suggest a variety of workarounds… e.g., you could rebind it to a variable of the same name (“Foo = Foo” before the if-statement), or add a NoQA for that case. But, of course, we shouldn’t be removing those.

I may just have to disable this behavior (of marking redefined-while-unused imports as unused) until we have more extensive branch analysis.

[charliermarsh]

(I probably won’t be able to resolve this today just given my schedule but hopefully a NoQA will be ok for you for now?)

[Czaki]

Do you mean add noqua to the import line? It may be ok.

name conditionally redefined should not be removed.

[charliermarsh]

Yeah, I mean adding a noqa to the import line, to avoid marking it as unused.

[Czaki]

I'm ok with this solution

[Czaki]

I try to move problematic imports under if clause and i could create commit that pass pre-commit but when run pre-commit on all files then ruff creates multiple warnings:

ruff.....................................................................Failed
- hook id: ruff
- exit code: 1

warning: `U` has been remapped to `UP`
warning: `U` has been remapped to `UP`
warning: `U` has been remapped to `UP`
warning: `U` has been remapped to `UP`
warning: `U` has been remapped to `UP`
warning: `U` has been remapped to `UP`
warning: `U` has been remapped to `UP`

https://results.pre-commit.ci/run/github/166421141/1671109943.V-T5M9gkRhOblSlqIfxcUA

That hides real error (in this case E501) because the output is truncated.

[charliermarsh]

@Czaki - Can you try upgrading? I made some improvements to how noqa are handled in those cases and modified the warnings to only omit once per code (you can just change U to UP in your pyproject.toml though hopefully to suppress them entirely).

[Czaki]

I see that pre-commit hook is in version 186 and this repository is in 188. Did I correctly understand that I need to use at least 188 to test?

[charliermarsh]

188 is now up. (There’s a slight delay between the two repos.)

[Czaki]

What is the expected change in behavior? noqa in both lines preventing removal?

[charliermarsh]

Yeah, if you apply this diff, Ruff won't touch that member:

diff --git a/foo.py b/foo.py
index d72d8d6e..d5d7a367 100644
--- a/foo.py
+++ b/foo.py
@@ -7,7 +7,11 @@ import numpy as np
 import packaging.version
 
 from PartSegCore.mask_create import MaskProperty
-from PartSegCore.project_info import AdditionalLayerDescription, HistoryElement, ProjectInfoBase
+from PartSegCore.project_info import (
+    AdditionalLayerDescription,
+    HistoryElement,
+    ProjectInfoBase,  # noqa: F401
+)
 from PartSegCore.roi_info import ROIInfo
 from PartSegCore.utils import numpy_repr
 from PartSegImage import Image

[charliermarsh]

Closing for now, but let me know if you continue to run into this issue.

[Czaki]

Thanks ❤️

[charliermarsh]

You're welcome :)