Skip to content

chore: improved type annotation - #360

Closed
jackgene wants to merge 2 commits into
lidatong:masterfrom
jackgene:feat/improved-type-annotation
Closed

jackgene wants to merge 2 commits into
lidatong:masterfrom
jackgene:feat/improved-type-annotation

Conversation

@jackgene

Copy link
Copy Markdown

This resolves #336

And also makes the dataclass_json.core.Json type a little stricter.

@jackgene jackgene changed the title Feat/improved type annotation chore: improved type annotation May 24, 2022
Comment thread dataclasses_json/core.py
_issubclass_safe)

Json = Union[dict, list, str, int, float, bool, None]
Json = Union[Dict[str, 'Json'], List['Json'], str, int, float, bool, None]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mypy does not support cyclic type aliases (python/mypy#731, python/typing#182).

error: Cannot resolve name "Json" (possible cyclic definition)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@andersk

andersk commented Aug 14, 2022

Copy link
Copy Markdown
Contributor

Please don’t merge this, it doesn’t work.

@jackgene

Copy link
Copy Markdown
Author

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.

@lidatong lidatong left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jackgene jackgene closed this by deleting the head repository Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exclude function type signature does not match usage

3 participants