Ele 381 add mypy to the cli repo - step 1#837
Conversation
ELE-381 Add MyPy to the CLI repo
We use type hints extensively in our python code, but we have a lot of errors that are currently untested in the CI. We want to add MyPy to our CI process - but that also requires fixing the existing errors we have. This is something the longer we wait with the harder it will become. DoD:
|
|
👋 @haritamar |
| allow_redefinition = True | ||
|
|
||
| [mypy-alive_progress] | ||
| ignore_missing_imports = True |
There was a problem hiding this comment.
For every library that didn't have a "stubs" library I needed to do this (See the libs I added to dev-requirements.txt)
| Receives a list of strings, either JSON dumped or not, dedups and sorts it, and returns it as a comma-separated | ||
| string. | ||
| This is useful for various lists we include in slack messages (owners, subscribers, etc) | ||
| """ |
There was a problem hiding this comment.
I removed prettify_json_str_set because it was confusing.
Made this function always return a string because that's the intention (it's for the owner list in slack messages)
haritamar
force-pushed
the
ele-381-add-mypy-to-the-cli-repo
branch
from
93faac8 to
d3dfbf0
Compare
| return [] | ||
| result = [] | ||
| for model_error_alert in model_error_alert_list: | ||
| assert isinstance(model_error_alert, ModelAlert) |
There was a problem hiding this comment.
How could a non ModelAlert be in this list in the first place?
Maybe we should make a check and raise an error?
There was a problem hiding this comment.
The problem is that self._components_to_alerts is a dict whose value changes according to the provided key. There's no way that I can tell to make mypy understand it.
Changed to an exception though.
There was a problem hiding this comment.
(may worth refactoring, but I want to avoid it for now)
elongl
left a comment
elongl
left a comment
There was a problem hiding this comment.
I think we should perhaps use the base classes in the typings rather than cast() to the parent classes. Additionally we might use Generics for that. I'd also avoid using assert as Python recommends to only use that keyword for tests and debugging purposes.
2 participants
No description provided.