mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Allow `__init__` and `__new__` to return `NoReturn`

Open BvB93 opened this issue 4 years ago • 13 comments

Closes https://github.com/python/mypy/issues/10342

This PR allows __init__ and __new__ methods to return NoReturn, allowing for the annotation of classes that cannot be directly initialized (e.g. those that are defined in C such as types.GeneratorType).

BvB93 avatar Sep 28 '21 15:09 BvB93

Any idea what's going on with the failed Run mypy_primer / Run (2) Test? It looks like it crashed while installing depencies or something.

BvB93 avatar Sep 30 '21 09:09 BvB93

@BvB93 try to close / open this PR. It should be fixed now 👍

sobolevn avatar Sep 30 '21 09:09 sobolevn

Let's give it a shot (note: I might need approval again as a first-time contributor)

BvB93 avatar Sep 30 '21 09:09 BvB93

@BvB93 sorry, I cannot approve it 😞 We need to wait for maintainers to do that.

sobolevn avatar Sep 30 '21 10:09 sobolevn

I pushed the approval button.

JelleZijlstra avatar Sep 30 '21 14:09 JelleZijlstra

So, are there any further comments on this PR?

BvB93 avatar Oct 11 '21 19:10 BvB93

Today I discovered that I can't annotate a class as unable to be instantiated through __init__() -> NoReturn. I'd love to see this PR merged!

benkehoe avatar Jan 19 '22 21:01 benkehoe

Please let me get something straight. So this PR proposes to be able to annotate the return type of a class' __init__() with NoReturn rather than None?

So like

class MyExampleClass:
    def __init__(self) -> NoReturn:
        pass

rather than

class MyExampleClass:
    def __init__(self) -> None:
        pass

If that's the case, I am very much welcoming the change. It is more understandable to read that a class' __init__ doesn't return anything (NoReturn) as oppose to read that it returns None (which techically is not true).

PedanticHacker avatar Feb 11 '22 13:02 PedanticHacker

Closing and re-opening as no CI runs appear to be starting.

BvB93 avatar Feb 11 '22 14:02 BvB93

Closing and re-opening as no CI runs appear to be starting.

Right, that's better. This PR does still need approval after the latest master branch rebase.

Please let me get something straight. So this PR proposes to be able to annotate the return type of a class' init() with NoReturn rather than None?

@PedanticHacker not quite, NoReturn is used as return-annotation for functions that always raise an exception. In the examples you provided None is still the appropriate return type, as python functions without an explicit return statement alwways return None (note that this is actual runtime behavior; not just a convention).

class MyExampleClass:
    def __init__(self) -> NoReturn:
        raise TypeError("Cannot initialize 'MyExampleClass'")

BvB93 avatar Feb 11 '22 14:02 BvB93

Rebased to get rid of another merge conflict. For future reference, is there a preference for either rebasing or updating from master when dealing with merge conflicts?

BvB93 avatar Feb 21 '22 15:02 BvB93

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

steam.py (https://github.com/Gobot1234/steam.py)
- steam/ext/commands/converters.py:494: error: "__new__" must return a class instance (got "NoReturn")  [misc]
- steam/ext/commands/commands.py:224: error: The type "Type[Greedy[Any]]" is not generic and not indexable  [misc]
+ steam/ext/commands/commands.py:224: error: Value of type "Callable[[VarArg(Any), KwArg(Any)], NoReturn]" is not indexable  [index]

github-actions[bot] avatar Feb 21 '22 15:02 github-actions[bot]

Rebased to get rid of another merge conflict. For future reference, is there a preference for either rebasing or updating from master when dealing with merge conflicts?

Merging is generally preferred. When you force-push, the review history for comments before the force-push gets lost in the UI, making it harder for reviewers to review previous changes.

JelleZijlstra avatar Feb 21 '22 15:02 JelleZijlstra

Hi @BvB93 @hauntsaninja, I made another PR which merged master, maybe it's worth to have a look.

iyanging avatar Aug 22 '22 15:08 iyanging

This got merged in #13480

hauntsaninja avatar Aug 23 '22 07:08 hauntsaninja

Thank you BvB93!

hauntsaninja avatar Aug 23 '22 07:08 hauntsaninja