add typehints to misc files #10994
add typehints to misc files #10994
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces Python type hints across several miscellaneous modules, improving code clarity and consistency.
- Added parameter and return type annotations to various model and helper methods
- Removed deprecated parameters and methods (
iaflag inget_waitinglist_size,get_status,is_waiting_for) - Ensured return paths include explicit
Nonewhere appropriate
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| openlibrary/plugins/upstream/models.py | Added return type hints to get_name, get_edit_count, etc. |
| openlibrary/core/waitinglist.py | Added return type hint to get_waitinglist_for_book |
| openlibrary/core/models.py | Broad addition of type hints; removed some unused/deprecated APIs |
Comments suppressed due to low confidence (2)
openlibrary/core/models.py:341
- Removing the
iaparameter is a breaking change and may affect existing callers. Consider deprecating the old signature or providing a migration path.
def get_waitinglist_size(self) -> int:
openlibrary/core/models.py:885
- The
get_statusmethod was removed, which may break existing functionality that depends on it. Ensure callers are updated or reintroduce it if still needed.
def get_usergroups(self):
| name = property(get_name) | ||
|
|
||
| def get_edit_history(self, limit=10, offset=0): | ||
| def get_edit_history(self, limit: int = 10, offset: int = 0): |
There was a problem hiding this comment.
This method is missing a return type annotation. Consider adding a return type hint, e.g., -> list[Version] or the appropriate return type from web.ctx.site.versions().
Copilot uses AI. Check for mistakes.
|
|
||
|
|
||
| def get_waitinglist_for_book(book_key): | ||
| def get_waitinglist_for_book(book_key) -> list[WaitingLoan]: |
There was a problem hiding this comment.
Add a type hint for the book_key parameter (e.g., book_key: str) to maintain consistency and clarity.
| def get_waitinglist_for_book(book_key) -> list[WaitingLoan]: | |
| def get_waitinglist_for_book(book_key: str) -> list[WaitingLoan]: |
Copilot uses AI. Check for mistakes.
| return None | ||
|
|
||
| def _get_lists(self, limit=50, offset=0, sort=True): | ||
| def _get_lists(self, limit: int = 50, offset: int = 0, sort: bool = True): |
There was a problem hiding this comment.
This helper lacks a return type annotation. You could add -> list[str] to indicate it returns a list of keys.
| def _get_lists(self, limit: int = 50, offset: int = 0, sort: bool = True): | |
| def _get_lists(self, limit: int = 50, offset: int = 0, sort: bool = True) -> list[Any]: |
Copilot uses AI. Check for mistakes.
| return usergroup in [g.key for g in self.usergroups] | ||
|
|
||
| def is_subscribed_user(self, username): | ||
| def is_subscribed_user(self, username: str) -> int: |
There was a problem hiding this comment.
The return type is annotated as int, but PubSub.is_subscribed likely returns a boolean. Update the annotation to -> bool to match the actual return value.
Copilot uses AI. Check for mistakes.
| return self._site.things(q) | ||
|
|
||
| def new_list(self, name, description, seeds, tags=None): | ||
| def new_list(self, name: str, description: str, seeds, tags=None): |
There was a problem hiding this comment.
Consider adding a return type annotation for this method (e.g., -> dict) to document what new_list returns.
| def new_list(self, name: str, description: str, seeds, tags=None): | |
| def new_list(self, name: str, description: str, seeds, tags=None) -> Any: |
Copilot uses AI. Check for mistakes.
| return self.key.split("/")[-1] | ||
|
|
||
| def preferences(self, use_store=False): | ||
| def preferences(self, use_store: bool = False): |
There was a problem hiding this comment.
This method is missing a return type annotation (e.g., -> dict[str, str]). Adding it will improve consistency with other type hints.
Copilot uses AI. Check for mistakes.
|
copilot's suggestions are all fine (maybe) but this PR isn't meant to fix up the files totally it's more of just adding a few small wins that I saw while browsing |
Closes #
I added a few here before focusing on other files but wanted to send these up for review too.
Technical
Testing
Screenshot
Stakeholders