-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix mypy error with dataclasses #701
Conversation
Had do ignore since mypy expect a DataClassIstance that don't exist
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #701 +/- ##
=======================================
Coverage 62.21% 62.21%
=======================================
Files 69 69
Lines 6076 6076
Branches 642 642
=======================================
Hits 3780 3780
Misses 2144 2144
Partials 152 152 ☔ View full report in Codecov by Sentry. |
@@ -75,7 +75,7 @@ def to_json(o: Any): | |||
elif hasattr(o, "dict"): # Pydantic | |||
return o.dict() | |||
elif is_dataclass(o): | |||
return dataclass_as_dict(o) | |||
return dataclass_as_dict(o) # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the error and isn't there a more elegant way to solve the issue ?
# type: ignore
looks a bit like "force-ignore-this".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And shouldn't the return type be defined for this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hoh That's explained in the PR description why I had to force ignore.
error was
src/aleph/vm/utils/__init__.py:78: error: No overload variant of "asdict" matches argument type "type[DataclassInstance]" [call-overload]
src/aleph/vm/utils/__init__.py:78: note: Possible overload variants:
src/aleph/vm/utils/__init__.py:78: note: def asdict(obj: DataclassInstance) -> dict[str, Any]
src/aleph/vm/utils/__init__.py:78: note: def [_T] asdict(obj: DataclassInstance, *, dict_factory: Callable[[list[tuple[str, Any]]], _T]) -> _T
Found 1 error in 1 file (checked 71 source files)
DataclassInstance is not something that exist, so for me it looks like a mypy bug. But if you have another fix I would be happy to accept it.
@Psycojoker which function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@olethanh github doesn't allow me to comment on any line but the exact function you are editing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it probably should but it's totally outside the scope of this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ended up doing it here #704
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merged #704
I'm a bit confused, I can't reproduce it locally, is it using "hatch run lint:typing"? Or with a specific version of mypy or on another code base or PR? |
It was one the solana branch, which added the dep : |
Had do ignore since mypy expect a DataClassIstance that don't exist