Skip to content

Conversation

tyralla
Copy link
Collaborator

@tyralla tyralla commented Oct 17, 2025

I think no one has complained so far. I just encountered this (in my understanding) lack in TypeChecker.is_noop_for_reachability working on #20068.

@tyralla tyralla requested a review from A5rocks October 17, 2025 21:16
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

yarl (https://github.com/aio-libs/yarl)
+ yarl/_url.py:550: error: Unused "type: ignore" comment  [unused-ignore]

@tyralla tyralla requested a review from hauntsaninja October 18, 2025 04:30
Copy link
Collaborator

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

I think this makes sense to me, even if it complicates the general idea of what exactly is exempt from unreachability warnings. (I guess now it's "anyway to bail out is OK"?)

I can also see this being unnecessary :^). Given nobody has complained, I'm not sure we really need to add this as a feature (since that's ultimately what this is).

Copy link
Collaborator

@sterliakov sterliakov left a comment

Choose a reason for hiding this comment

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

I think this is a good idea. See #363 (yes, three-digit issue number!) - this removes an unreachable warning from the "should be fine" snippet there. I guess it has never been explicitly reported only because --warn-unreachable is hidden too well, many users aren't even aware that it exists.

@tyralla
Copy link
Collaborator Author

tyralla commented Oct 19, 2025

I think this is a good idea. See #363 (yes, three-digit issue number!) - this removes an unreachable warning from the "should be fine" snippet there.

Wow, I overlooked this dinosaur. That's a nice tool you have!

Interpreting NotImplemented as Any seems not to be the only problem. I just played around a little. Mypy does not predict the following crash:

class A1:
    x = 1
    def __add__(self, o: A1) -> float:
        if isinstance(o, A1):
            return self.x + o.x
        return NotImplemented

class A2:
    x = 2
    def __add__(self, o: A2) -> float:
        return self.x + o.x

class B:
    y = 3
    def __radd__(self, o: A1 | A2) -> float:
        return self.y + o.x

A1() + B()  # works
A2() + B()  # fails  (undetected by Mypy)

I will try to find a fix for this as soon as I have some spare time.

I guess it has never been explicitly reported only because --warn-unreachable is hidden too well, many users aren't even aware that it exists.

Yes, I agree. As soon as --warn-unreachable will be included in --strict, it will likely be discussed more frequently.

I think this makes sense to me, even if it complicates the general idea of what exactly is exempt from unreachability warnings. (I
I can also see this being unnecessary :^). Given nobody has complained, I'm not sure we really need to add this as a feature (since that's ultimately what this is).

It would say it is more of a fix than a feature, because return NotImplemented is reachable, and so the current warning is wrong.

@tyralla
Copy link
Collaborator Author

tyralla commented Oct 19, 2025

Thanks for the reviews!

@ilevkivskyi ilevkivskyi merged commit c2a82b9 into python:master Oct 19, 2025
20 checks passed
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.

4 participants