Skip to content

Docs: adds a note about issubclass usage in common_issues.rst #11014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 4, 2021
Merged

Docs: adds a note about issubclass usage in common_issues.rst #11014

merged 3 commits into from
Sep 4, 2021

Conversation

sobolevn
Copy link
Member

It was really not clear for some users that you can use issubclass as well.

It was really not clear for some users that you can use `issubclass` as well.
@sobolevn
Copy link
Member Author

@terencehonles oh! Thanks! I make this typo all the time! 😆

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks for all your work to improve mypy docs. I'm not sure this is the right section: I don't think issubclass is a common solution and I'd like to try and keep the common issues page focussed.

Also, looking more carefully, I believe mypy does now narrow for type(o) is int, so this whole section feels out of date...

Anyway, I think what I'd want on the "Common issues" page is a description of how to use assert and cast to convince mypy that something is of a given type.

Maybe we can add a new top level section on narrowing with detailed examples like in this PR (that we could then link to from common issues). I remember us struggling to find a home for the excellent TypeGuard documentation you wrote, a narrowing section could be a good place for that too.

@sobolevn
Copy link
Member Author

sobolevn commented Sep 3, 2021

Anyway, I think what I'd want on the "Common issues" page is a description of how to use assert and cast to convince mypy that something is of a given type.

Sure! I will change this PR to add cast and assert.

Also, looking more carefully, I believe mypy does now narrow for type(o) is int, so this whole section feels out of date...

Yes, it works for type(o) is ... case. But, it does not work for the case in my example:

def f(o: object) -> None:
    if issubclass(type(o), MyCalcMeta): 
        reveal_type(type(o))  # Revealed type is "Type[builtins.object]"

Should I open a new issue to track this?

Maybe we can add a new top level section on narrowing with detailed examples like in this PR (that we could then link to from common issues)

Sounds good to me! I will create one more PR later.

@sobolevn
Copy link
Member Author

sobolevn commented Sep 4, 2021

Moreover, current mypy docs here are outdated:

  1. This statement:

Mypy can’t infer the type of o after the type() check because it only knows about isinstance() (and the latter is better style anyway)

This is not true anymore.

 def f(o: object) -> None:
       if type(o) is int:
           o = cast(int, o)
           g(o + 1)    # This would be an error without the cast
           ...
       else:
           ...

In this example cast is no longer required. The revealed type of o is Revealed type is "builtins.int".

I am going to change this example completely. I will find some really useful usecases for cast() that cannot be modeled with isinstance, issubclass, or with custom typeguards.

Things I've done:
1. Changed an outdated example to an original one from python/typing#15 (comment)
2. I've listed more type narrowing techniques, like `type(obj) is some_class`
3. Fixed several headers

.. code-block:: python
You can read more about type narrowing techniques here.
Copy link
Member Author

Choose a reason for hiding this comment

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

I will add a link to the new page here in the next PR

Mypy can usually infer the types correctly when using :py:func:`isinstance <isinstance>`,
:py:func:`issubclass <issubclass>`,
or ``type(obj) is some_class`` type tests,
and even user-defined type guards,
Copy link
Member Author

Choose a reason for hiding this comment

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

I will add a link here as well in the next PR

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Awesome, this looks great!

@hauntsaninja hauntsaninja merged commit b17ae30 into python:master Sep 4, 2021
@sobolevn
Copy link
Member Author

sobolevn commented Sep 4, 2021

@hauntsaninja thanks a lot for your help! 👍

I will return with the next PR somewhere this week.

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.

3 participants