Cleanup @property stuff on BaseRemoteEndpoint #136
Conversation
pipermerriam
changed the title
| def is_running(self) -> bool: | ||
| return not self.is_stopped and self.running.is_set() | ||
| return not self.is_stopped and self._running.is_set() |
There was a problem hiding this comment.
I'm really confused by this. How is it possible that neither mypy nor flake8 caught this?
There was a problem hiding this comment.
In my local env. just pushed something to see if CI catches it.
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, it seems it catches it after this change but not on current master which is really weird. Also, the @property decorators also need to be applied on the RemoteEndpointAPI.
There was a problem hiding this comment.
In fact, as soon as you remove the @property from is_stopped it won't catch this error anymore. This feels like a weird mypy bug to me.
There was a problem hiding this comment.
Ok, here is a minimal test case.
class TestCase:
def foo(self) -> bool:
return False and self.meh()This is not caught by mypy. Do you think this is expected behavior?
There was a problem hiding this comment.
I opened an issue against mypy python/mypy#6990
pipermerriam
force-pushed
the
piper/cleanup-RemoteEndpoint-properties
branch
from
cd08697
to
42ec907
Compare
What was wrong?
There were some errors in the implementation that needed cleaning up
How was it fixed?
Fixed them (need to add tests)
Cute Animal Picture