make mypy succeed in strict mode #14
Conversation
|
Hi @autra, Thank you for the updates and the workflow addition! Thanks again! |
| _settings: CloudEventSettings = CloudEventSettings() | ||
| _json: Any | ||
| _body: Any | ||
| _body: bytes |
There was a problem hiding this comment.
@autra, I checked what happens if this is replaced by Any and # type: ignore at the end of the body function the strict mypy still passes.
What do you think?
I'm not sure if I'm missing something.
Would it be ok if we keep it Any, and just ignore away the other warnings caused by this?
I don't want to break anybody depending on it.
There was a problem hiding this comment.
I don't think this change (from Any to bytes) really breaks anything that wasn't broken before. If I understand correctly, Any basically means "don't bother checking" to the type checker. If we change it to bytes, we align ourselves with the reality (because the runtime type is bytes already, whether we type check or not).
For instance, with this snippet, the type checker is happy, but we still have a failure at runtime:
from typing import Any
body: Any = b'foo'
body.decode("UTF-8")
body.append("foo")If we change the type of body to bytes, same error, but at type checking time. I don't think failing faster should be considered as a breaking change. Actually, people using type checking tool would arguably want that. What do you think?
There was a problem hiding this comment.
I dont feel comfertable to chainging this type to bytes.
I think the slight upside is not worth the risk of messing some downstream dependencies for reasons I cannot forsee.
@autra Would you be open to changing it back to Any?
There was a problem hiding this comment.
Well of course, it's your call after all.
That being said, what about releasing a major version then, if you're afraid about potential breakage? People won't automatically get it and it should be ok.
But again, we're just changing the type hint, the only thing we can ever break is mypy execution, not real execution.
If you still want me to revert that, no problem I'll do it.
There was a problem hiding this comment.
@autra Let's keep it Any.
We will change it to bytes on the next Major release
Thank you autra so much!
|
@autra Also, don't worry about CI, I have checked it locally |
|
@autra Thank you so much for your contribution Version will be released in the following days |
2 participants
Hi!
I noticed the project didn't pass mypy in strict mode, so I went ahead and fixed it.
I also added a github workflow that checks the typing on every pull request, if you're interested.
Thanks!