Skip to content

Improve library typing#147

Draft
CrisMystik wants to merge 16 commits into
KurimuzonAkuma:devfrom
CrisMystik:dev
Draft

Improve library typing#147
CrisMystik wants to merge 16 commits into
KurimuzonAkuma:devfrom
CrisMystik:dev

Conversation

@CrisMystik

@CrisMystik CrisMystik commented Feb 26, 2025

Copy link
Copy Markdown
Contributor

This pull request aims to improve Pyrogram/Kurigram's type hinting through the following changes:

  • Type-hint all "public" fields and methods (previously, some of them were missing a type annotation)
  • Fix some typing errors, for example: many fields were like parameter: Union[X, Y] = None, which is incorrect (it should be Union[X, Y, None])
  • Add type-hints for handlers and decorators, and filters
  • Add a basic type-hint for progress callbacks

Other changes that I still have not made:

  • Make all raw.types.* classes inherit from their respective raw.base.* classes - and mark the latter as abstract classes already being worked on by feat(cod-gen): change raw types generation to have proper typehints #130 apparently
  • Find a better way to annotate functions that return a different type based on their parameters (e.g. download, get_users)
  • Add type hinting for invoke() using generics

@KurimuzonAkuma

Copy link
Copy Markdown
Owner

parameter: Union[X, Y] = None, which is incorrect (it should be Union[X, Y, None])

i prefer Optional[Union[X, Y]]

Comment thread pyrogram/client.py
from mimetypes import MimeTypes
from pathlib import Path
from typing import Union, List, Optional, Callable, AsyncGenerator, Type, Tuple
from typing import Any, Union, List, Optional, Callable, AsyncGenerator, Type, Tuple, Optional

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

there are two Optional

self,
value: Union[int, str, raw.types.RpcError] = None,
rpc_name: str = None,
value: Union[int, str, raw.types.RpcError, None] = None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think it should be Optional[Union[int, str, raw.types.RpcError]]

@Danipulok

Danipulok commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

Make all raw.types.* classes inherit from their respective raw.base.* classes - and mark the latter as abstract classes already being worked on by #130 apparently

#130 was closed in favor of #151 that was merged today.
Also, you shouldn't change a few things in the same PR.

Find a better way to annotate functions that return a different type based on their parameters (e.g. download, get_users)

Again, should be a separate PR. You can also add get_user (I've thought about this for a while), and use @typing.overload for download function. And maybe add @typing.overload to get_users too, since it's used a lot.

Add type hinting for invoke() using generics

I don't quite think there's a way to do this via generics. But maybe there's a hack. Most probably it'd be a separate generator function that creates file with @typing.overload for all possible invoke outcomes.

@Danipulok

Copy link
Copy Markdown
Contributor

@CrisMystik why is it draft, though?
What's here to be done?

@Danipulok

Copy link
Copy Markdown
Contributor

@CrisMystik are you still willing to finish this?

@KurimuzonAkuma

Copy link
Copy Markdown
Owner

@CrisMystik are you still willing to finish this?

я живой?

@Danipulok

Copy link
Copy Markdown
Contributor

@CrisMystik are you still willing to finish this?

я живой?

надеюсь.

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