🚨 Add and check static type hints using mypy#65
Conversation
hf-kklein
changed the title
hf-krechan
left a comment
hf-krechan
left a comment
There was a problem hiding this comment.
Thank you for implementing mypy
I just have one two questions.
| from marshmallow_enum import EnumField | ||
| from bo4e.com.rufnummer import Rufnummer, RufnummerSchema | ||
| from bo4e.com.adresse import Adresse, AdresseSchema | ||
| from marshmallow_enum import EnumField # type:ignore |
There was a problem hiding this comment.
we have to check the enum topic again. I think they are not well implemented.
There was a problem hiding this comment.
In this specific line the problem is the marshmallow_enum package itself.
| def _create_empty_referenzen_list() -> List[ExterneReferenz]: | ||
| """ | ||
| A method with a type hint to please mypy | ||
| https://stackoverflow.com/a/61281305/10009545 | ||
| :return: | ||
| """ | ||
| return [] |
There was a problem hiding this comment.
I do not get the connection between this function and the linked SO.
They are talking about Union issue. but here is no Union.
Maybe I am to tired to see it.
There was a problem hiding this comment.
the thing is that the empty list [] (of type List[Any]) does not have the type expected here. That's why I'm "hiding" the empty list returned in a function that has a type hint that matches mypys expectations. This idea is copied from the "Approach 2 - Use a type-hinted function for empty list assignment]" of the linked SO answer.
| jobtitel: str = fields.Str(missing=None) | ||
| abteilung: str = fields.Str(missing=None) | ||
| jobtitel = fields.Str(missing=None) | ||
| abteilung = fields.Str(missing=None) |
There was a problem hiding this comment.
What is the reason that the typehint is not needed here?
There was a problem hiding this comment.
it was in fact wrong here, because we're not in die class Zustaendigkeit itself but in the corresponding schema whose fields are not strings but fields. The previously wrong type hint here is actually one of the errors that mypy found in our code.
| Anrede = Enum( | ||
| "Anrede", | ||
| { | ||
| "HERR": "HERR", | ||
| "FRAU": "FRAU", | ||
| "EHELEUTE": "EHELEUTE", | ||
| "FIRMA": "FIRMA", | ||
| "INDIVIDUELL": "INDIVIDUELL", | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Oh ah, I see it's nicer. Did your IDE you any other tool gave you a hint?
There was a problem hiding this comment.
at least mypy does now, because the code is not really executed (at least not multiple lines like f.e. dictionary initialization and its usage as argument to Enum())
2 participants
Lesson learned: Enums are hard to type. But the way I constructed it now both the tests run and there's no need to ignore the enums during type check.
You need to update the branch protection rules to merge this. First remove the linting status check, then re-add both of the linters.