Skip to content

MAINT: Adding ruff unsafe-fixes - #236

Open
prady0t wants to merge 2 commits into
data-apis:mainfrom
prady0t:ruff-unsafe-fixes
Open

MAINT: Adding ruff unsafe-fixes#236
prady0t wants to merge 2 commits into
data-apis:mainfrom
prady0t:ruff-unsafe-fixes

Conversation

@prady0t

@prady0t prady0t commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Towards #228

prady0t added 2 commits July 30, 2026 22:45
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>
Comment on lines +28 to +31
try:
from typing import Self
except ImportError: # Python < 3.11
from typing_extensions import Self

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any better way to do this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH this whole -> Array -to- -> Self thingy is a very typical example of two kinds of nonsense: one from typing and another from linting.

This is all make-work. It's make-work to create a patch, it's make-work to review, it's make-work to even think about any of this. Why oh why does anyone spends any part of their waking time actually thinking about whether an __iadd__ returns an Array or Self? Why does this whole contraption asks us to read about PEP673 --- okay, I googled it, I skimmed through it, fine; however long it took me, it's wasted time --- I'm ready to believe that somebody somewhere somehow cares about this and there exists at least one situation where it matters for whatever definition of matter, okay. But why on good green earth, why does it get inflicted on us?

Anyhow, I'm done venting.
Short answer: from my perch, I don't care, as long as this does not get in the way (as in, I'll probably have to pip install something next time I do actual work in an environment with python==3.10, OK, I'll do it once and move on doing actual work).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough 😄.

@ev-br ev-br left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to understand why these are unsfafe-fixes. The changes seem to be of two groups:

  • typing -> Array replaced by -> Self (ok whatever)
    This should be perfectly safe at runtime
  • nested ifs collapsed with either and or isinstance(object, tuple_of_types)
    The former form shortcuts fine, the latter... probably shortcuts too, not sure.
    Plus benign refactors of if-else into a dict.get etc.

And if it's labeled as unsafe, it means I don't see something, so what is it I'm not seeing: Which of these is unsafe and how?

Comment on lines +28 to +31
try:
from typing import Self
except ImportError: # Python < 3.11
from typing_extensions import Self

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH this whole -> Array -to- -> Self thingy is a very typical example of two kinds of nonsense: one from typing and another from linting.

This is all make-work. It's make-work to create a patch, it's make-work to review, it's make-work to even think about any of this. Why oh why does anyone spends any part of their waking time actually thinking about whether an __iadd__ returns an Array or Self? Why does this whole contraption asks us to read about PEP673 --- okay, I googled it, I skimmed through it, fine; however long it took me, it's wasted time --- I'm ready to believe that somebody somewhere somehow cares about this and there exists at least one situation where it matters for whatever definition of matter, okay. But why on good green earth, why does it get inflicted on us?

Anyhow, I'm done venting.
Short answer: from my perch, I don't care, as long as this does not get in the way (as in, I'll probably have to pip install something next time I do actual work in an environment with python==3.10, OK, I'll do it once and move on doing actual work).

@prady0t

prady0t commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

I'd like to understand why these are unsfafe-fixes.

See: https://docs.astral.sh/ruff/linter/#fix-safety

I think for the first part: the conditional import is something that I added manually; originally Ruff just imported Self from typing. But for older versions, it's only available in typing_extensions. And the CI did fail on that.

For the second part, I'm not sure about it either.

@ev-br

ev-br commented Jul 30, 2026

Copy link
Copy Markdown
Member

Thanks for the link. My question is still why these fixes are "unsafe" in the meaning of that link: what are the changes to the runtime behaviour.

@prady0t

prady0t commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

After a bit more digging through the docs:

This rule's fix is marked as unsafe as it changes the meaning of your type annotations.

When preview is enabled, the fix for this rule is considered as safe. When preview is not enabled, the fix is always considered unsafe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants