Conversation
…eted types (#89) Updated `get_stories` and `MessageStory._parse` to correctly reflect that they can return `StorySkipped` and `StoryDeleted` types, resolving `invalid-return-type` errors reported by the `ty` type checker.
* Fix unknown-argument type errors This commit resolves all "unknown-argument" errors reported by the ty type checker. Key changes include: - Fixed Star Gift related methods in business methods to use correct raw API parameters and types. - Updated account.UpdateColor call to use PeerColor object. - Switched to messages.DeleteScheduledMessages for scheduled message deletion. - Removed unsupported parameters in send_media_group and send_paid_media. - Fixed CallbackQuery.edit_message_caption to call correct client methods and accept business_connection_id. - Removed unused/incorrect parameters in DraftMessage and get_chat calls.
Signed-off-by: 5hojib <yesiamshojib@gmail.com>
…rs (#91) This commit addresses several type errors reported by the `ty` type checker: - Fixed `invalid-return-type` in `delete_scheduled_messages.py`. - Resolved all 34 `invalid-assignment` errors by using `setattr`/`getattr` in decorators and using separate variables for narrowed types in methods. - Modernized `compiler/api/compiler.py` to use PEP 604 `| None` syntax instead of `Optional[...]`, and regenerated all raw API files. - Cleaned up temporary files and experimental test code.
Signed-off-by: 5hojib <yesiamshojib@gmail.com>
…mprovements (#93) * fix(ty): resolve several invalid-argument-type errors - Update `ChatInviteLink._parse` and `Chat._parse_chat` to accept `None` for optional data. - Fix `dict.get()` calls with potentially `None` keys in `ChatJoinRequest` and `ChatJoiner`. - Filter `None` values from entity lists in `Message._parse`. - Update `Message.markdown` and `Message.html` to handle missing entities. - Fix `Message.click` to correctly access `button.text`. - Update `EditMessageCaption` and `EditMessageMedia` to accept `None` for `invert_media`. - Fix API compiler to generate valid type hints for forward references using PEP 604 syntax. - Add `from __future__ import annotations` to generated raw API files. - Use `list` instead of `typing.List` in raw API templates. * fix(ty): resolve several invalid-argument-type errors - Update `ChatInviteLink._parse` and `Chat._parse_chat` to accept `None` for optional data. - Fix `dict.get()` calls with potentially `None` keys in `ChatJoinRequest` and `ChatJoiner`. - Filter `None` values from entity lists in `Message._parse`. - Update `Message.markdown` and `Message.html` to handle missing entities. - Fix `Message.click` to correctly access `button.text`. - Update `EditMessageCaption` and `EditMessageMedia` to accept `None` for `invert_media`. - Fix API compiler to generate valid type hints for forward references using PEP 604 syntax. - Add `from __future__ import annotations` to generated raw API files. - Use `list` instead of `typing.List` in raw API templates.
Signed-off-by: 5hojib <yesiamshojib@gmail.com>
Contributor
Reviewer's GuideRefactors type hint generation and tightens many type- and None-safety edge cases across messages, chats, stories, business payments, and low-level RPC helpers, while aligning several methods with updated Telegram APIs and fixing small logic bugs in search, deletion, and UI helpers. Sequence diagram for updated callback_query.edit_message_caption flowsequenceDiagram
actor User
participant BotClient
participant CallbackQuery
participant TelegramAPI
User->>BotClient: presses_inline_button
BotClient->>CallbackQuery: handler_calls_edit_message_caption(caption, parse_mode, reply_markup, business_connection_id)
alt has_message_id_and_not_inline
CallbackQuery->>BotClient: edit_message_caption_via_client(chat_id, message_id, caption, parse_mode, reply_markup, business_connection_id_or_message_business_connection_id)
BotClient->>TelegramAPI: editMessageCaption(request_with_business_connection_id)
TelegramAPI-->>BotClient: edited_message_or_bool
BotClient-->>CallbackQuery: result
else has_inline_message_id
CallbackQuery->>BotClient: edit_inline_caption(inline_message_id, caption, parse_mode, reply_markup)
BotClient->>TelegramAPI: editInlineMessageCaption(request)
TelegramAPI-->>BotClient: bool
BotClient-->>CallbackQuery: result
end
CallbackQuery-->>BotClient: edited_message_or_bool
BotClient-->>User: updated_message_caption_visible
Sequence diagram for delete_scheduled_messages single vs multiple idssequenceDiagram
actor User
participant Client
participant TelegramAPI
User->>Client: delete_scheduled_messages(chat_id, message_ids)
Client->>Client: resolve_peer(chat_id)
alt message_ids_is_int
Client->>Client: is_iterable = False
Client->>Client: ids = [message_ids]
else message_ids_is_iterable
Client->>Client: is_iterable = True
Client->>Client: ids = list(message_ids)
end
Client->>TelegramAPI: DeleteScheduledMessages(peer, ids)
TelegramAPI-->>Client: success_ack
alt is_iterable
Client-->>User: ids_list
else
Client-->>User: single_id
end
Class diagram for updated story parsing and retrievalclassDiagram
class Client {
+get_stories(chat_id int_or_str, story_ids int_or_iterable) StoryOrSkippedOrDeletedOrListOrNone
}
class MessageMediaStory {
}
class MessageStory {
+from_user
+sender_chat
+_parse(client Client, message_story MessageMediaStory) MessageStoryOrStoryOrSkippedOrDeletedOrListOrNone
}
class Story {
}
class StorySkipped {
}
class StoryDeleted {
}
Client "1" --> "many" Story : returns
Client "1" --> "many" StorySkipped : returns
Client "1" --> "many" StoryDeleted : returns
MessageStory --> MessageMediaStory : wraps
MessageStory --> Story : may_return
MessageStory --> StorySkipped : may_return
MessageStory --> StoryDeleted : may_return
%% Type aliases (expressed as pseudo methods for clarity)
class StoryOrSkippedOrDeletedOrListOrNone {
}
class MessageStoryOrStoryOrSkippedOrDeletedOrListOrNone {
}
Client --> StoryOrSkippedOrDeletedOrListOrNone : get_stories
MessageStory --> MessageStoryOrStoryOrSkippedOrDeletedOrListOrNone : _parse
Class diagram for updated business star gift operationsclassDiagram
class Client {
+get_user_gifts(user_id int_or_str, offset int, limit int) StarGiftsPage
+send_gift(chat_id int_or_str, gift_id int, is_private bool, text str, entities list_MessageEntity) PaymentResult
+sell_gift(chat_id int_or_str, message_id int) PaymentResult
+toggle_gift_is_saved(chat_id int_or_str, message_id int, is_saved bool) PaymentResult
}
class GetSavedStarGifts {
+peer
+offset
+limit
}
class SaveStarGift {
+stargift InputSavedStarGiftUser
+unsave bool
}
class ConvertStarGift {
+stargift InputSavedStarGiftUser
}
class InputSavedStarGiftUser {
+msg_id int
}
class InputInvoiceStarGift {
+peer
+gift_id int
+hide_name bool
+message TextWithEntities
}
class TextWithEntities {
+text str
+entities list_MessageEntity
}
class MessageEntity {
}
class StarGiftsPage {
}
class PaymentResult {
}
Client --> GetSavedStarGifts : invokes
Client --> SaveStarGift : invokes
Client --> ConvertStarGift : invokes
Client --> InputInvoiceStarGift : creates
SaveStarGift --> InputSavedStarGiftUser : uses
ConvertStarGift --> InputSavedStarGiftUser : uses
InputInvoiceStarGift --> TextWithEntities : uses
TextWithEntities --> MessageEntity : has
GetSavedStarGifts --> StarGiftsPage : returns
SaveStarGift --> PaymentResult : returns
ConvertStarGift --> PaymentResult : returns
InputInvoiceStarGift --> PaymentResult : part_of_send_gift_response
Client --> StarGiftsPage : get_user_gifts
Client --> PaymentResult : send_gift
Client --> PaymentResult : sell_gift
Client --> PaymentResult : toggle_gift_is_saved
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Summary of ChangesHello @5hojib, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modernizes the codebase by adopting contemporary Python type hinting practices, enhancing the compiler's type generation capabilities, and updating several Telegram API interaction methods to reflect recent changes. It also includes various refinements to message and chat object parsing, improving overall code robustness and clarity. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
CallbackQuery.edit_message_caption, thebusiness_connection_idargument is only forwarded for non-inline messages; forinline_message_idthe value is ignored, which is inconsistent withedit_message_textand likely drops business-specific context. - In
messages.send_media_group, theschedule_repeat_periodargument is no longer passed to the underlying request, so callers setting this parameter will silently see no effect; consider re‑wiring it through or removing the parameter from the public API. - In
search_global_hashtag_messages,offset_dateis now unused after introducingoffset_rate; either removeoffset_dateentirely or keep the variable names aligned to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CallbackQuery.edit_message_caption`, the `business_connection_id` argument is only forwarded for non-inline messages; for `inline_message_id` the value is ignored, which is inconsistent with `edit_message_text` and likely drops business-specific context.
- In `messages.send_media_group`, the `schedule_repeat_period` argument is no longer passed to the underlying request, so callers setting this parameter will silently see no effect; consider re‑wiring it through or removing the parameter from the public API.
- In `search_global_hashtag_messages`, `offset_date` is now unused after introducing `offset_rate`; either remove `offset_date` entirely or keep the variable names aligned to avoid confusion.
## Individual Comments
### Comment 1
<location> `pyrogram/types/user_and_chats/chat.py:773-776` </location>
<code_context>
- chat: raw.types.Chat | raw.types.User | raw.types.Channel,
+ chat: raw.types.Chat | raw.types.User | raw.types.Channel | None,
) -> Chat | None:
+ if chat is None:
+ return None
+
if isinstance(chat, raw.types.Chat | raw.types.ChatForbidden):
return Chat._parse_chat_chat(client, chat)
if isinstance(chat, raw.types.User):
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `isinstance` with a PEP 604 union (`A | B`) will raise at runtime.
`isinstance(chat, raw.types.Chat | raw.types.ChatForbidden)` will raise `TypeError` at runtime because `isinstance` doesn’t accept PEP 604 unions. Use a tuple instead: `isinstance(chat, (raw.types.Chat, raw.types.ChatForbidden))` to preserve the intended behavior.
</issue_to_address>
### Comment 2
<location> `compiler/api/compiler.py:91` </location>
<code_context>
def get_type_hint(type: str) -> str:
- is_flag = FLAGS_RE.match(type)
+ is_flag = bool(FLAGS_RE.match(type))
</code_context>
<issue_to_address>
**issue (complexity):** Consider splitting type resolution from formatting by extracting a `get_base_hint` helper that returns both the base hint and whether it is a `raw.base` type, simplifying `get_type_hint`’s control flow and string handling.
You can keep the new behavior (PEP 604 unions, `raw.base` handling) while simplifying flow by separating “type resolution” from “formatting”, and by avoiding substring checks on `"raw.base"`.
For example:
```py
def get_type_hint(type: str) -> str:
is_flag = bool(FLAGS_RE.match(type))
if is_flag:
type = type.split("?")[1]
hint, is_raw_base = get_base_hint(type)
if is_flag:
if is_raw_base:
return f'"{hint} | None" = None'
return f"{hint} | None = None"
if is_raw_base:
return f'"{hint}"'
return hint
def get_base_hint(t: str) -> tuple[str, bool]:
if t in CORE_TYPES:
if t == "long" or "int" in t:
return "int", False
if t == "double":
return "float", False
if t == "string":
return "str", False
if t in {"Bool", "true"}:
return "bool", False
return "bytes", False
if t in {"Object", "!X"}:
return "TLObject", False
if re.match("^vector", t, re.IGNORECASE):
sub_type = t.split("<", 1)[1][:-1]
sub_hint, _ = get_base_hint(sub_type)
return f"list[{sub_hint}]", False
ns, name = t.split(".") if "." in t else ("", t)
return "raw.base." + ".".join([ns, name]).strip("."), True
```
This keeps all new semantics but:
- Removes nested function inside `get_type_hint`.
- Encodes `raw.base` as explicit state (`is_raw_base`) instead of string inspection.
- Keeps quoting/`| None` logic in a single, small, post-processing block.
</issue_to_address>
### Comment 3
<location> `pyrogram/types/messages_and_media/message.py:1133` </location>
<code_context>
- for entity in (message.entities or [])
- ]
- entities = types.List(filter(lambda x: x is not None, entities))
+ entities = types.List(
+ [
+ e
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the new nested list comprehensions for `entities` and `quote_entities` with straightforward loops that build filtered lists in a single pass for better readability and debuggability.
The new nested list comprehensions for `entities` and `quote_entities` make the logic harder to follow without changing behavior. You can keep the explicit filtering of `None` while simplifying to a single pass that’s easier to read and debug.
For `entities`:
```python
# Current
entities = types.List(
[
e
for e in [
types.MessageEntity._parse(client, entity, users)
for entity in (message.entities or [])
]
if e is not None
],
)
# Suggested
parsed_entities = []
for entity in (message.entities or []):
e = types.MessageEntity._parse(client, entity, users)
if e is not None:
parsed_entities.append(e)
entities = types.List(parsed_entities)
```
For `quote_entities`:
```python
# Current
if message.reply_to.quote_entities:
parsed_message.quote_entities = types.List(
[
e
for e in [
types.MessageEntity._parse(client, entity, users)
for entity in message.reply_to.quote_entities
]
if e is not None
],
)
# Suggested
if message.reply_to.quote_entities:
parsed_quote_entities = []
for entity in message.reply_to.quote_entities:
e = types.MessageEntity._parse(client, entity, users)
if e is not None:
parsed_quote_entities.append(e)
parsed_message.quote_entities = types.List(parsed_quote_entities)
```
This keeps behavior intact, preserves the explicit `None` filtering, avoids double iteration, and removes the nested comprehension structure that other reviewers flagged as complex.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+773
to
776
| if chat is None: | ||
| return None | ||
|
|
||
| if isinstance(chat, raw.types.Chat | raw.types.ChatForbidden): |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Using isinstance with a PEP 604 union (A | B) will raise at runtime.
isinstance(chat, raw.types.Chat | raw.types.ChatForbidden) will raise TypeError at runtime because isinstance doesn’t accept PEP 604 unions. Use a tuple instead: isinstance(chat, (raw.types.Chat, raw.types.ChatForbidden)) to preserve the intended behavior.
| @@ -89,32 +89,40 @@ def camel(s: str): | |||
|
|
|||
|
|
|||
| def get_type_hint(type: str) -> str: | |||
Contributor
There was a problem hiding this comment.
issue (complexity): Consider splitting type resolution from formatting by extracting a get_base_hint helper that returns both the base hint and whether it is a raw.base type, simplifying get_type_hint’s control flow and string handling.
You can keep the new behavior (PEP 604 unions, raw.base handling) while simplifying flow by separating “type resolution” from “formatting”, and by avoiding substring checks on "raw.base".
For example:
def get_type_hint(type: str) -> str:
is_flag = bool(FLAGS_RE.match(type))
if is_flag:
type = type.split("?")[1]
hint, is_raw_base = get_base_hint(type)
if is_flag:
if is_raw_base:
return f'"{hint} | None" = None'
return f"{hint} | None = None"
if is_raw_base:
return f'"{hint}"'
return hint
def get_base_hint(t: str) -> tuple[str, bool]:
if t in CORE_TYPES:
if t == "long" or "int" in t:
return "int", False
if t == "double":
return "float", False
if t == "string":
return "str", False
if t in {"Bool", "true"}:
return "bool", False
return "bytes", False
if t in {"Object", "!X"}:
return "TLObject", False
if re.match("^vector", t, re.IGNORECASE):
sub_type = t.split("<", 1)[1][:-1]
sub_hint, _ = get_base_hint(sub_type)
return f"list[{sub_hint}]", False
ns, name = t.split(".") if "." in t else ("", t)
return "raw.base." + ".".join([ns, name]).strip("."), TrueThis keeps all new semantics but:
- Removes nested function inside
get_type_hint. - Encodes
raw.baseas explicit state (is_raw_base) instead of string inspection. - Keeps quoting/
| Nonelogic in a single, small, post-processing block.
| for entity in (message.entities or []) | ||
| ] | ||
| entities = types.List(filter(lambda x: x is not None, entities)) | ||
| entities = types.List( |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider replacing the new nested list comprehensions for entities and quote_entities with straightforward loops that build filtered lists in a single pass for better readability and debuggability.
The new nested list comprehensions for entities and quote_entities make the logic harder to follow without changing behavior. You can keep the explicit filtering of None while simplifying to a single pass that’s easier to read and debug.
For entities:
# Current
entities = types.List(
[
e
for e in [
types.MessageEntity._parse(client, entity, users)
for entity in (message.entities or [])
]
if e is not None
],
)
# Suggested
parsed_entities = []
for entity in (message.entities or []):
e = types.MessageEntity._parse(client, entity, users)
if e is not None:
parsed_entities.append(e)
entities = types.List(parsed_entities)For quote_entities:
# Current
if message.reply_to.quote_entities:
parsed_message.quote_entities = types.List(
[
e
for e in [
types.MessageEntity._parse(client, entity, users)
for entity in message.reply_to.quote_entities
]
if e is not None
],
)
# Suggested
if message.reply_to.quote_entities:
parsed_quote_entities = []
for entity in message.reply_to.quote_entities:
e = types.MessageEntity._parse(client, entity, users)
if e is not None:
parsed_quote_entities.append(e)
parsed_message.quote_entities = types.List(parsed_quote_entities)This keeps behavior intact, preserves the explicit None filtering, avoids double iteration, and removes the nested comprehension structure that other reviewers flagged as complex.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a substantial number of improvements across the codebase, aligning with recent Telegram API changes, enhancing type safety, and fixing several bugs. The refactoring in the API compiler and the robustness improvements in various parsers are particularly noteworthy. The changes make the library more reliable and maintainable. I have a couple of suggestions for minor refactoring to further improve code readability.
Comment on lines
+1133
to
+1142
| entities = types.List( | ||
| [ | ||
| e | ||
| for e in [ | ||
| types.MessageEntity._parse(client, entity, users) | ||
| for entity in (message.entities or []) | ||
| ] | ||
| if e is not None | ||
| ], | ||
| ) |
Contributor
There was a problem hiding this comment.
This nested list comprehension can be a bit difficult to read. For improved readability and to make it more idiomatic, you could use a single list comprehension with an assignment expression (walrus operator), which is available in Python 3.8+.
entities = types.List(
parsed_entity
for entity in message.entities or []
if (parsed_entity := types.MessageEntity._parse(client, entity, users)) is not None
)
Comment on lines
1524
to
1533
| parsed_message.quote_entities = types.List( | ||
| filter(lambda x: x is not None, quote_entities), | ||
| [ | ||
| e | ||
| for e in [ | ||
| types.MessageEntity._parse(client, entity, users) | ||
| for entity in message.reply_to.quote_entities | ||
| ] | ||
| if e is not None | ||
| ], | ||
| ) |
Contributor
There was a problem hiding this comment.
Similar to the previous comment on parsing entities, this nested list comprehension for quote_entities could be simplified for better readability using an assignment expression.
parsed_message.quote_entities = types.List(
parsed_entity
for entity in message.reply_to.quote_entities
if (
parsed_entity := types.MessageEntity._parse(
client, entity, users
)
)
is not None
)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Align Pyrogram with recent Telegram API changes, improve type handling and robustness, and fix various edge-case bugs across messages, chats, stories, and business payments.
Bug Fixes:
Enhancements:
Build:
Documentation:
Chores: