Add some typehints in core/helpers.py #9788
Conversation
openlibrary/core/helpers.py
Outdated
| import babel | ||
| import babel.core | ||
| import babel.dates | ||
| import babel.numbers | ||
|
|
||
| from babel.core import Locale | ||
| from typing import Optional |
There was a problem hiding this comment.
| from typing import Optional |
openlibrary/core/helpers.py
Outdated
| @@ -196,7 +212,7 @@ def sprintf(s, *a, **kw): | |||
| return s | |||
|
|
|||
|
|
|||
| def cond(pred, true_value, false_value=""): | |||
| def cond(pred, true_value, false_value): | |||
There was a problem hiding this comment.
| def cond(pred, true_value, false_value): | |
| def cond(pred: Any, true_value: Any, false_value: Any = "") -> Any: |
Good call on not putting list[Author] here, etc., as that could be misleading, given all the possible valid things one could supply here.
My suggestion, using Any, is of questionable value, but it least it lets others know the function has been considered vis-a-vis typing.
The main thing is that we'll want to keep the "" default value for false_value, so we don't make false_value a required argument for calling functions that may now rely on it having a default.
openlibrary/core/helpers.py
Outdated
| if TYPE_CHECKING: | ||
| from openlibrary.plugins.upstream.models import Author | ||
|
|
There was a problem hiding this comment.
| if TYPE_CHECKING: | |
| from openlibrary.plugins.upstream.models import Author |
Because Author isn't used, this won't be needed.
openlibrary/core/helpers.py
Outdated
| from typing import TYPE_CHECKING | ||
|
|
There was a problem hiding this comment.
| from typing import TYPE_CHECKING |
Because Author isn't used, this won't be needed.
|
Thank you for the informative feedback! I'll add all of these changes |
Related to #8028
Adds typehints in core/helpers.py
Technical
Testing
Screenshot
Stakeholders
@scottbarnes