Member

Basically a long-time result of microsoft/pylance-release#1983: Explicity adds type annotations to attributes of classes.
While I still don't really like that there is a (partial) need for this, it's mostly a one-time effort.
Almost all additions where ofc automated. It's "only" type hints, so if I got something wrong here, it doesn't have runtime effect, but a "slow-scrolling-quick-read" review would still be good at some point :D

Todo:

• check with pyright --verifytypes telegram what that tool thinks about the type completeness of our library and check what we can improve about it
• think about adding a step to the CI that ensures that type completeness only ever increases
• check in vscode if the types are better now

Member Author

Small update: Apparently pyright still thinks that PTb is not very type complete:

{
        "exportedSymbolCounts": {
            "withKnownType": 415,
            "withAmbiguousType": 30,
            "withUnknownType": 294
        },
        "otherSymbolCounts": {
            "withKnownType": 1815,
            "withAmbiguousType": 702,
            "withUnknownType": 566
        },
        "missingFunctionDocStringCount": 25,
        "missingClassDocStringCount": 0,
        "missingDefaultParamCount": 107,
        "completenessScore": 0.5615696887686062,
}

Full json output.

Member Author

Update:

{
        "exportedSymbolCounts": {
            "withKnownType": 426,
            "withAmbiguousType": 277,
            "withUnknownType": 36
        },
        "otherSymbolCounts": {
            "withKnownType": 2292,
            "withAmbiguousType": 671,
            "withUnknownType": 120
        },
        "missingFunctionDocStringCount": 25,
        "missingClassDocStringCount": 0,
        "missingDefaultParamCount": 107,
        "completenessScore": 0.5764546684709067,
}

So at least a rather large jump from "unknown" to "ambiguous - even though the completeness increased only ~1.5 points 😄
I found out that as soon as anything in a class A is unknown/ambiguous (annotation of param/return value/attr, base class), that whole class A is of unknown/ambiguous type and using this class A in some other class B as type annotation will hence also make B unknown/ambiguous.

Also I apparently messed up the tests big time

Member Author

{
        "exportedSymbolCounts": {
            "withKnownType": 705,
            "withAmbiguousType": 0,
            "withUnknownType": 34
        },
        "otherSymbolCounts": {
            "withKnownType": 3180,
            "withAmbiguousType": 0,
            "withUnknownType": 102
        },
        "missingFunctionDocStringCount": 23,
        "missingClassDocStringCount": 0,
        "missingDefaultParamCount": 147,
        "completenessScore": 0.9539918809201624,
}

😎

Member Author

Last update for today messe up the tests, but got to 96%:

Symbols exported by "telegram": 734
  With known type: 707
  With ambiguous type: 0
  With unknown type: 27
    (Ignoring unknown types imported from other packages)
  Functions without docstring: 13
  Functions without default param: 0
  Classes without docstring: 0
Other symbols referenced but not exported by "telegram": 2496
  With known type: 2408
  With ambiguous type: 1
  With unknown type: 87
Type completeness score: 96.3%

IISC most of what's left boils down to the generic nature of Application and CallbackContext. I'll see how far I'll get with that …

Member Author

The GH Actions thingy doesn't quite work yet and sphinx is acting up while documenting ContextTypes.DEFAULT_TYPES. but type completeness is at 100% now 🥳
If anyone's using VSCode + pylance, I'd be happy to know if this improves autocompletion etc noticable :)

Member

Okay so I tested it and it's just as good as before, except now the autocompletion of context.{user, chat,bot}_data works better. Also sadly it still shows None for something like update.message.poll.id, though I guess we can't fix that without a TypeGuard(?).

Anyway running pyright on my machine still found some unknown types, sending a pastebin in dev chat

Member Author

@harshil21 Nice :)
I doubt that TypeGuard can help with that. To know in which handlers update.message.poll is None and in which it's a Message, we'd have to make Update and Message generic classes and couple that with the handlers & filters - (nearly) impossible

which Python version are did you run pyright on? did you use the --ignoreexternal flag?

Member

Choose a reason for hiding this comment

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

Member Author

Is it worth to also modify bot method parameters to Optional[...] as well? Even the newer mypy version complains of implicit optionals now

You mean e.g. parse_mode: Optional[str] = None instead of parse_mode: str = None? Does mypy copmlain about that when you use PTB or only when developing PTB?

Member

You mean e.g. parse_mode: Optional[str] = None instead of parse_mode: str = None?

yes

Does mypy copmlain about that when you use PTB or only when developing PTB?

pylance complains about that while developing PTB. I mentioned mypy since they made a similar change with implicit_optional

Member Author

pylance complains about that while developing PTB.

So this changed? #3367 (comment)

If it has: Fair enough, let's do it. But I vote to do that in a different PR - we can reopen #3367 for that.

Choose a reason for hiding this comment

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