Skip to content
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

chore: improved type annotation #360

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions dataclasses_json/cfg.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
import functools
from enum import Enum
from typing import Callable, Dict, Optional, TypeVar, Union
from typing import Callable, Dict, Optional, Union

from marshmallow.fields import Field as MarshmallowField

from dataclasses_json.core import Json
from dataclasses_json.stringcase import (camelcase, pascalcase, snakecase,
spinalcase) # type: ignore
from dataclasses_json.undefined import Undefined, UndefinedParameterError

T = TypeVar("T")


class Exclude:
"""
Pre-defined constants for exclusion. By default, fields are configured to
be included.
"""
ALWAYS: Callable[[T], bool] = lambda _: True
NEVER: Callable[[T], bool] = lambda _: False
ALWAYS: Callable[[Json], bool] = lambda _: True
NEVER: Callable[[Json], bool] = lambda _: False


# TODO: add warnings?
Expand Down Expand Up @@ -51,16 +50,16 @@ class LetterCase(Enum):
PASCAL = pascalcase


def config(metadata: dict = None, *,
def config(metadata: Optional[dict] = None, *,
# TODO: these can be typed more precisely
# Specifically, a Callable[A, B], where `B` is bound as a JSON type
encoder: Callable = None,
decoder: Callable = None,
mm_field: MarshmallowField = None,
encoder: Optional[Callable] = None,
decoder: Optional[Callable] = None,
mm_field: Optional[MarshmallowField] = None,
letter_case: Union[Callable[[str], str], LetterCase, None] = None,
undefined: Optional[Union[str, Undefined]] = None,
field_name: str = None,
exclude: Union[Callable[[str, T], bool], Exclude, None] = None,
undefined: Union[str, Undefined, None] = None,
field_name: Optional[str] = None,
exclude: Union[Callable[[Json], bool], Exclude, None] = None,
) -> Dict[str, dict]:
if metadata is None:
metadata = {}
Expand Down
4 changes: 2 additions & 2 deletions dataclasses_json/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from datetime import datetime, timezone
from decimal import Decimal
from enum import Enum
from typing import Any, Collection, Mapping, Union, get_type_hints, Tuple
from typing import Any, Collection, Dict, List, Mapping, Union, get_type_hints, Tuple
from uuid import UUID

from typing_inspect import is_union_type # type: ignore
Expand All @@ -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]
Copy link
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
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
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
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
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.


confs = ['encoder', 'decoder', 'mm_field', 'letter_case', 'exclude']
FieldOverride = namedtuple('FieldOverride', confs)
Expand Down