-
Notifications
You must be signed in to change notification settings - Fork 40
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
Allow bytes in from_json typing #471
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #471 +/- ##
=======================================
Coverage 89.82% 89.82%
=======================================
Files 12 12
Lines 1937 1937
Branches 401 401
=======================================
Hits 1740 1740
Misses 145 145
Partials 52 52 ☔ View full report in Codecov by Sentry. |
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.
Thanks! @davetapley Please take a look at my comments.
serde/de.py
Outdated
@@ -390,7 +391,7 @@ class Deserializer(Generic[T], metaclass=abc.ABCMeta): | |||
|
|||
@classmethod | |||
@abc.abstractmethod | |||
def deserialize(cls, data: T, **opts: Any) -> Any: | |||
def deserialize(cls, data: AnyStr, **opts: Any) -> Any: |
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.
Can you leave it unchanged as it may be non AnyStr in the future?
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.
Good point. Done (amended)!
15880f0
to
919c1ab
Compare
919c1ab
to
e493deb
Compare
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.
LGTM!
Fixes #450