Conversation
| self.name: str | None = None | ||
| self.description: str | None = None | ||
| self.bot_commands: list[BotCommand] = None | ||
| self.bot_commands: list[BotCommand] | None = None |
There was a problem hiding this comment.
Do we need default None here?
Looks like default should be [ ]
There was a problem hiding this comment.
Pull request overview
This PR updates type hints across the aiomax library to better reflect actual runtime values (notably timestamps and optional returns), and fixes a small typo in an attachment error message.
Changes:
- Annotate message payload builders and bot/router state with more explicit types.
- Adjust timestamp-related attributes to
float-based seconds and make several APIs returnMessage | None. - Fix typo in an attachment validation error string.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
aiomax/utils.py |
Adds Any and types the constructed message body dict; fixes an error-message typo. |
aiomax/types.py |
Updates various timestamp/value type hints and several from_json / message-related return types. |
aiomax/router.py |
Adds an explicit optional type for parent. |
aiomax/bot.py |
Types session and bot_commands as optional. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.last_activity_time: float | None = ( | ||
| last_activity_time / 1000 if last_activity_time else None | ||
| ) |
There was a problem hiding this comment.
float | None is a PEP 604 union and will be evaluated at runtime; this raises on Python 3.9. Use a quoted annotation or Optional[float] here. Also, last_access_time is still annotated as int | None but is computed via / 1000, which produces a float—its annotation should be updated for consistency.
| self.is_admin: "bool | None" = is_admin | ||
| self.join_time: "int | None" = join_time / 1000 if join_time else None | ||
| self.join_time: "float | None" = join_time / 1000 if join_time else None | ||
| self.permissions: "list[str] | None" = permissions |
There was a problem hiding this comment.
float | None is a PEP 604 union and will be evaluated at runtime; this raises on Python 3.9. Use a quoted annotation or Optional[float] for join_time (or bump the minimum Python version).
| def from_json(data: dict) -> "LinkedMessage | None": | ||
| if data is None: | ||
| return None |
There was a problem hiding this comment.
data can be None (the function explicitly handles that), but the parameter is typed as dict. Update the signature to accept dict | None (or equivalent) so callers like data.get(...) type-check cleanly.
| self.type: str = type | ||
| self.status: str = status | ||
| self.last_event_time: int = ( | ||
| self.last_event_time: float | None = ( |
There was a problem hiding this comment.
float | None is a PEP 604 union and will be evaluated at runtime; this raises on Python 3.9. Use a quoted annotation or Optional[float] for last_event_time (or bump the minimum Python version).
| self.last_event_time: float | None = ( | |
| self.last_event_time: Optional[float] = ( |
| ] = {} # commands in this router | ||
| self.case_sensitive: bool = case_sensitive | ||
| self.parent = None # Parent bot of this router | ||
| self.parent: Router | None = None # Parent bot of this router |
There was a problem hiding this comment.
self.parent: Router | None uses PEP 604 unions, which are evaluated at runtime and will raise on Python 3.9 (the project declares requires-python >= 3.9). Use a quoted annotation (e.g. "Router | None") or Optional[Router] to keep 3.9 compatibility (or bump the minimum Python version if 3.10+ is required).
| self.parent: Router | None = None # Parent bot of this router | |
| self.parent: Optional["Router"] = None # Parent bot of this router |
| ] = {} # commands in this router | ||
| self.case_sensitive: bool = case_sensitive | ||
| self.parent = None # Parent bot of this router | ||
| self.parent: Router | None = None # Parent bot of this router |
There was a problem hiding this comment.
The inline comment says “Parent bot of this router”, but parent is typed/used as a parent router (and can be a Bot only in the root case). Update the comment to reflect what parent actually represents to avoid confusion when working with nested routers.
| self.parent: Router | None = None # Parent bot of this router | |
| self.parent: Router | None = None # Parent router in the hierarchy (may be the bot for the root router) |
|
|
||
| self.access_token: str = access_token | ||
| self.session = None | ||
| self.session: aiohttp.ClientSession | None = None |
There was a problem hiding this comment.
aiohttp.ClientSession | None is a PEP 604 union and will be evaluated at runtime; this raises on Python 3.9. Since the project targets 3.9+, use a quoted annotation or Optional[aiohttp.ClientSession] (or raise the minimum supported Python version).
| self.name: str | None = None | ||
| self.description: str | None = None | ||
| self.bot_commands: list[BotCommand] = None | ||
| self.bot_commands: list[BotCommand] | None = None |
There was a problem hiding this comment.
list[BotCommand] | None is a PEP 604 union and will be evaluated at runtime; this raises on Python 3.9. Use a quoted annotation or Optional[list[BotCommand]] (or bump requires-python to 3.10+).
|
|
||
| @staticmethod | ||
| def from_json(data: dict) -> "MessageRecipient": | ||
| def from_json(data: dict) -> "MessageRecipient | None": |
There was a problem hiding this comment.
data can be None (the function explicitly handles that), but the parameter is typed as dict. Update the signature to accept dict | None (or equivalent) so callers like data.get(...) type-check cleanly.
| def from_json(data: dict) -> "MessageRecipient | None": | |
| def from_json(data: "dict | None") -> "MessageRecipient | None": |
| def from_json(data: dict) -> "MessageBody | None": | ||
| if data is None: | ||
| return None |
There was a problem hiding this comment.
data can be None (the function explicitly handles that), but the parameter is typed as dict. Update the signature to accept dict | None (or equivalent) so callers like data.get(...) type-check cleanly.
Updated some type hints