chore: improved type annotation #360
Conversation
… of T since the types aren't really parameterized.
jackgene
changed the title
| @@ -23,7 +23,7 @@ | |||
| _is_optional, _isinstance_safe, | |||
| _issubclass_safe) | |||
|
|
|||
| Json = Union[dict, list, str, int, float, bool, None] | |||
| Json = Union[Dict[str, 'Json'], List['Json'], str, int, float, bool, None] | |||
There was a problem hiding this comment.
Mypy does not support cyclic type aliases (python/mypy#731, python/typing#182).
error: Cannot resolve name "Json" (possible cyclic definition)
There was a problem hiding this comment.
Looks like per your link (python/mypy#731, which linkes to python/mypy#13297 - towards the bottom), MyPy supports recursive types now. But looks like this was just implemented ~2 weeks ago, so I understand not wanting to rely on it yet.
Anyway, per my other comment, we've already forked this for our needs. I'm just going to leave this PR here for when you are comfortable that MyPy's support for recursive typing can be relied on.
There was a problem hiding this comment.
hm @jackgene is this a released version of mypy? if you're willing to update the PR and bump mypy as appropriate, i'm happy to merge (up to you)
There was a problem hiding this comment.
Let me test this a bit more with the new MyPy. Probably doesn't hurt to wait a bit. It probably won't be done today, but I'll stay on top of it. Thank you!
There was a problem hiding this comment.
It’s not in the current release (0.971); it’s still only in mypy Git, and only behind an experimental feature flag --enable-recursive-aliases.
|
Please don’t merge this, it doesn’t work. |
Yeah, no worries not merging. We've already forked this for our purposes (we use PyRight for type-checking, which does not suffer from MyPy's limitations). I'll leave this open for now, just in case MyPy figures out recursive typing at some point in the future. |
This resolves #336
And also makes the
dataclass_json.core.Jsontype a little stricter.