Fix mypy type errors #64

AdiNar · 2022-07-19T16:28:22Z

#51
I've look around the type errors that mypy found. Unfortunately it's harder than I thought - some of them are caused by the fact that some code is valid in dynamic context, but making it understandable by mypy is not trivial. Another group will require some changes in code. Yet another group of errors might probably be fixed by using TypeGuards, but this is a new feature from python3.10.

In this PR I fix almost all errors in parsers.py file. There is one left and fixing it will probably require more changes in code (Liskov principle is broken there, I described it more in the commit message).

More errors arise after the change, but as far as my changes make sense, they are caused by mypy having more data to analyze.

I attach mypy output before and after the changes and the diff between them. Checked on project virtualenv with python3.8, all test are passing.
mypy_diff.txt
mypy_new.txt
mypy_old.txt

From the code it seems that the M type should not be used (as it may be mistaken with one for Mappings). I've renamed it to META to avoid problems. Also, mypy complains about reassigning the same type variable, so there are META_ and META now.

Tuple[Field] describes only one element tuple. Tuple[Field, ...] describes tuple of any length.

* `N` type is used only to assert the returned variable type. There is no relationship that would justify it being the TypeVar, which has to be listed in the class definiton. Union type suits better. * `complex` type was removed from the type as it's never used in the project. It even causes problems during comparisons, as it's imcomparable with real numbers.

AdiNar · 2022-07-19T16:29:06Z

dataclass_wizard/abstractions.py

@@ -88,7 +88,7 @@ def list_to_json(cls: Type[W],


 @dataclass
-class AbstractParser(ABC):
+class AbstractParser(ABC, Generic[T, TT]):


Generic types must be listed in the class definition.

I also thought about introducing an intermediate layer for vast majority of the cases, that would be specified with T only (like SimpleAbstractParser(AbstractParser[Type[T], T])). However, it introduces some problems with __slots__ etc and might unnecessarily affect performance.

AdiNar · 2022-07-20T08:59:36Z

dataclass_wizard/abstractions.py

@@ -88,7 +88,7 @@ def list_to_json(cls: Type[W],


 @dataclass
-class AbstractParser(ABC):
+class AbstractParser(ABC, Generic[T, TT]):


We need two types, as there are some edge cases like in UnionParser or PatternedDTParser

AdiNar · 2022-07-20T09:04:51Z

dataclass_wizard/parsers.py

@@ -433,7 +433,7 @@ def __post_init__(self, cls: Type,
                      get_parser: GetParserType):

        # Get the field annotations for the `NamedTuple` type
-        type_anns: Dict[str, Type[T]] = get_named_tuple_field_types(
+        type_anns: Dict[str, Type[Any]] = get_named_tuple_field_types(


As i understand, annotations are the specified field types and there is no bound for their types

agreed 💯 . I guess I wasn't looking at it too closely when I decided to subscript like Type[T]. Or maybe I just copied what I was using for most things; but you're totally right on this one.

AdiNar · 2022-07-20T09:08:26Z

dataclass_wizard/type_def.py

@@ -72,7 +73,7 @@
 DD = TypeVar('DD', bound=DefaultDict)

 # Numeric type
-N = TypeVar('N', int, float, complex)
+N = Union[int, float]


As in commit message:

N type is used only to assert the returned variable type. There is no
relationship that would justify it being the TypeVar, which has to be listed in
the class definiton. Union type suits better.

complex type was removed from the type as it's never used in the project.
It even causes problems during comparisons, as it's imcomparable with real numbers.

* Add generic types to Parsers Generic types were used in the class body, but they also have to be specified in the class definition. * There is one error left - in the type of `DefaultDictParser` hook. It differs from what was defined in `MappingParser` and is seems that it breaks Liskov principle. I don't know how to fix that without some big changes in code.

AdiNar · 2022-07-20T19:48:43Z

dataclass_wizard/utils/typing_compat.py

@@ -376,7 +376,7 @@ def eval_forward_ref(base_type: FREF,
    return typing._eval_type(base_type, base_globals, _TYPING_LOCALS)


-def eval_forward_ref_if_needed(base_type: FREF,
+def eval_forward_ref_if_needed(base_type: typing.Union[typing.Type, FREF],


As I understand here, the if_needed stands for the fact that the argument doesn't have to be FREF, we check it in the if below.

totally right. I was also considering pulling out the if check and that way we could avoid an unnecessary function call most of the time, but I also do like it because all of that logic is abstracted away into a helper function. So i'm still debating what's the ideal approach, but you are right insofar as that base_type can either be a concrete type or a forward ref type, such as str for instance.

rnag · 2022-08-15T16:23:16Z

Hi @AdiNar, thanks again for opening this PR. First off, just wanted to say that I appreciate you taking the time and effort to put all this together. I understand that navigating the intricacies of this project's codebase can sometimes be a challenge and an uphill struggle, and other times just an exercise in futility because it seems like there is wall after wall to punch through before you finally break into the treasure room, where all the good stuff is at.

That said, I do have plans in the long-term to look into refactoring the codebase, to break it out and at the least to make it a little more digestible and easier to understand, and considerably easier to locate things so it feels less like spaghetti code and a bit more like everything's neater and more well rounded, so we're not looking at a half dozen files or modules to try to hunt down the cause of an obscure bug for example (yep, definitely been there). However that's still in the pipeline for now, and I'll likely add a milestone to help track that effort since I personally feel like it'll be worthwhile to knock down a pin or two to push that through the goalpost when time allows.

I do apologize for not looking at the PR till now, but I've been kept busy with life and other stuff and haven't been able to take an active interest in personal projects lately, though hopefully that will change starting soon. Last month (July) was in fact a pretty eventful month for me; I had a third row seat at a solid fireworks show, turned 30 for the first time ever - not a milestone to take lightly of course, found out I was left-handed at a sport I had never played before, attended my very first live orchestra event and coincidentally also sat through one of the original Star Wars movies that I'd only heard of in passing and in plenty of memes, and also came head-to-head with COVID for the first time ever. So all in all it was rather eventful, and though not all of it was good, I enjoyed my time and was caught up by events and the going ons for the most part.

All that out of the way, I do want to mention that I have some upcoming important changes and updates I am planning to push out rather soon, so depending on timing and other factors I might ask you to merge or pull in the changes once I have everything wrapped up and added to the main branch. Again, if it is too much work I can also set some time apart to address the mypy and type errors with the updated codebase, since I feel I have a better understanding of how it all comes together now.

Also, fingers crossed but I definitely forsee that hopefully by the end of the month these changes can be integrated in the project codebase, and we should be able to address type errors and warnings as this PR correctly indicates. In closing, I want to extend gratitude as I understand that it's not a trivial task to refactor the project to make it behave a little better with mypy and other similar type checkers - personally I don't use them, hence why you might have ended up discovering so many errors and linting issues with the current codebase 😅.

stdedos · 2023-11-14T11:55:41Z

Hello @rnag!

Since it has been 1y+ from your last comment, and:

PR is a step forward
PR is not "awfully complicated"

May I suggest merging it (also at least stdedos@2a50fb0#diff-848eae7420b3910167674508afab5eaba80b92ae43fd6bba5ddaea3e787cdf9f) and creating a new alpha/beta/rc/bugfix/minor release out of this?

rnag · 2024-11-04T03:54:50Z

@stdedos You are too right you friend, I have been too busy w/ life and forgot completely about this.

I will go ahead to merge, as you suggested.

I appreciate your hard work and effort, thank you, I personally do not use mypy but I understand others might stand to benefit from this.

That said, I'll verify with tests, and create a release shortly. Stay tuned for that. Thanks!

rnag · 2024-11-04T04:19:11Z

@AdiNar this is merged as part of v0.24.1

I credited you in the release notes. Thanks again for your work on this 🫡 .

AdiNar added 3 commits July 19, 2022 18:08

Rename Meta type from M to META

31eb046

From the code it seems that the M type should not be used (as it may be mistaken with one for Mappings). I've renamed it to META to avoid problems. Also, mypy complains about reassigning the same type variable, so there are META_ and META now.

Fix tuples types

89c9886

Tuple[Field] describes only one element tuple. Tuple[Field, ...] describes tuple of any length.

AdiNar commented Jul 19, 2022

View reviewed changes

AdiNar commented Jul 20, 2022

View reviewed changes

AdiNar force-pushed the fix-mypy-type-errors branch from 38cf8b3 to 8ce3c8c Compare July 20, 2022 19:41

AdiNar commented Jul 20, 2022

View reviewed changes

AdiNar marked this pull request as ready for review July 20, 2022 20:06

rnag merged commit aaa2cf6 into rnag:main Nov 4, 2024

rnag mentioned this pull request Nov 4, 2024

Fix module mypy static type check errors #51

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix mypy type errors #64

Fix mypy type errors #64

AdiNar commented Jul 19, 2022 •

edited

Loading

AdiNar Jul 19, 2022 •

edited

Loading

AdiNar Jul 20, 2022

AdiNar Jul 20, 2022

rnag Aug 15, 2022

AdiNar Jul 20, 2022 •

edited

Loading

AdiNar Jul 20, 2022 •

edited

Loading

rnag Aug 15, 2022

rnag commented Aug 15, 2022 •

edited

Loading

stdedos commented Nov 14, 2023

rnag commented Nov 4, 2024 •

edited

Loading

rnag commented Nov 4, 2024

Fix mypy type errors #64

Fix mypy type errors #64

Conversation

AdiNar commented Jul 19, 2022 • edited Loading

AdiNar Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

AdiNar Jul 20, 2022

Choose a reason for hiding this comment

AdiNar Jul 20, 2022

Choose a reason for hiding this comment

rnag Aug 15, 2022

Choose a reason for hiding this comment

AdiNar Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

AdiNar Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

rnag Aug 15, 2022

Choose a reason for hiding this comment

rnag commented Aug 15, 2022 • edited Loading

stdedos commented Nov 14, 2023

rnag commented Nov 4, 2024 • edited Loading

rnag commented Nov 4, 2024

AdiNar commented Jul 19, 2022 •

edited

Loading

AdiNar Jul 19, 2022 •

edited

Loading

AdiNar Jul 20, 2022 •

edited

Loading

AdiNar Jul 20, 2022 •

edited

Loading

rnag commented Aug 15, 2022 •

edited

Loading

rnag commented Nov 4, 2024 •

edited

Loading