Provide annotations for kombu/utils/encoding.py
#2208
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2208 +/- ##
==========================================
- Coverage 81.54% 81.52% -0.03%
==========================================
Files 77 77
Lines 9517 9527 +10
Branches 1154 1158 +4
==========================================
+ Hits 7761 7767 +6
Misses 1564 1564
- Partials 192 196 +4 ☔ View full report in Codecov by Sentry. |
The initial annotations provided aim to annotate the current implementation opposed to potentially refactoring to narrow down the original intention of the provided implementations.
for more information, see https://pre-commit.ci
Nusnus
force-pushed
the
annotate-utils-encoding
branch
from
4f51694 to
2a911cd
Compare
| def _safe_str( | ||
| s: object, | ||
| errors: str = 'replace', | ||
| file: Any = None |
There was a problem hiding this comment.
Some places say Any and some object.
Is there any specific reason for not using the same annotation all over?
There was a problem hiding this comment.
Apologies for the late response.
There are a number of differences between the annotations of typing.Any and object, but I don't think it really matters in any of the cases I provided. TLDR; I've updated all uses of object to typing.Any.
For functions like safe_repr() (shown below), technically, __str__() and __repr__() are functions on the object class (see here), but I can't think of any real-life examples where an object doesn't have a __repr__() function except maybe weird metaclasses or C extension modules.
def safe_repr(
o: Any,
errors: str = 'replace',
) -> str:
"""Safe form of repr, void of Unicode errors."""
try:
return repr(o)
except Exception:
return _safe_str(o, errors)I don't know the codebase that well, but I assume the purpose of functions like these are to catch errors raised by the __repr__() function rather than to catch an AttributeError in the case where an object doesn't have a repr. So does this mean there's value in the type-checker only accepting objects which have a __repr__()? Probably not, there's way more that could go wrong at runtime anyway.
Is there anything that should be done to get this PR mergable? I'm happy to close this if the project is no-longer accepting typing-only PRs, otherwise I think I have addressed the comments on the PR. |
Linked Issue: #1511
For this PR, I aimed to annotate the existing implementation of the encoding functions, regardless of what I thought the function's intention is.
There are some functions e.g.
str_to_bytes,bytes_to_str, orsafe_strwhich I think may have potentially misleading implementations. I'm not sure whether this is under the scope of this PR, input on this would be appreciated :).