Preliminary implementation of stdlib/3.7/dataclasses.pyi. #1944

gwk · 2018-03-06T02:37:56Z

This is a first effort at adding typing for the 3.7 dataclasses module.

JelleZijlstra · 2018-03-06T02:50:46Z

stdlib/3.7/dataclasses.py

@@ -0,0 +1,47 @@
+from typing import *


Thanks for adding these stubs!

A couple of style comments:

don't use import *

names that don't actually exist at runtime (like T and DictType) should be private, so their names should start with an underscore.

indent with four spaces

add spaces after : in annotations and around = for annotated functions with arguments.

gwk · 2018-03-26T16:36:24Z

@JelleZijlstra thanks for the feedback; working on an update now. Does the private rule include public names imported from typing? from typing import Type or from typing import Type as _Type?

JelleZijlstra · 2018-03-26T17:03:52Z

Names imported with from A import B are not public; those imported with from A import B as C (or from A import B as B) are, unless their name starts with an underscore.

gwk · 2018-03-26T17:10:29Z

I made the requested style changes, and one important fix: dataclass is now typed as an overloaded function, for the uncalled @dataclass and called @dataclass(...) cases.

JelleZijlstra · 2018-03-26T19:12:54Z

There are still a few Travis failures that should be fixable.

gwk · 2018-03-27T17:59:17Z

@JelleZijlstra latest push passed all tests - please give it another look!

sproshev · 2018-03-28T11:34:40Z

stdlib/3.7/dataclasses.pyi

+
+@overload
+def dataclass(*, init: bool = ..., repr: bool = ..., eq: bool = ...,
+  order: bool = ..., hash: Optional[bool] = ..., frozen: bool = ...) -> Callable[[Type[_T]], Type[_T]]: ...


unsafe_hash, see https://www.python.org/dev/peps/pep-0557/#id7

Thanks. The hash tristate parameter changed to unsafe_hash boolean in cpython commit dbf9cff48a4ad0fd58e1c623ce1f36c3dd3d5f38.

JelleZijlstra

I reviewed the code against the implementation of dataclasses.py (not the documentation since it doesn't exist yet: https://bugs.python.org/issue32216) and noticed a couple of places where it can be improved.

Also cc @ericvsmith who implemented dataclasses and @ilevkivskyi who is interested in writing dataclass support in mypy.

JelleZijlstra · 2018-03-29T01:51:37Z

stdlib/3.7/dataclasses.pyi

+_DictType = TypeVar('_DictType', bound=dict)
+_TupleType = TypeVar('_TupleType', bound=tuple)
+
+class _MISSING_TYPE: ...


We're missing MISSING.

JelleZijlstra · 2018-03-29T01:57:45Z

stdlib/3.7/dataclasses.pyi

+
+def field(*, default: Union[_T, _MISSING_TYPE] = ..., default_factory: Union[Callable[[], _T], _MISSING_TYPE] = ...,
+    init: bool = ..., repr: bool = ..., hash: Optional[bool] = ..., compare: bool = ...,
+    metadata: Optional[Mapping[str, Any]] = ...) -> Field: ...


Comments in the code say

# This function is used instead of exposing Field creation directly, # so that a type checker can be told (via overloads) that this is a # function whose type depends on its parameters.

I do think we should use overloads. An overload with default given should return a Field[_T] and allow default_factory as an argument, and one with default not given or _MISSING_TYPE should not allow default_factory and return Field[Any].

An overload with default given should return a Field[_T] and allow default_factory as an argument

I think you mean "disallow default_factory as an argument", correct?

Oops yes, they are exclusive.

As I read it, there are three cases: neither provided (which returns Field[Any]), just default provided, and just default_factory provided. The last two both return Field[_T]. Is that correct?

That sounds right.

JelleZijlstra · 2018-03-29T02:00:13Z

stdlib/3.7/dataclasses.pyi

+    init: bool = ..., repr: bool = ..., hash: Optional[bool] = ..., compare: bool = ...,
+    metadata: Optional[Mapping[str, Any]] = ...) -> Field: ...
+
+def fields(class_or_instance: Type) -> Tuple[Field, ...]: ...


This also accepts dataclass instances. We should probably just type it as Any; the mypy plugin can do something smarter internally if we want to.

Also, we should be explicit here and say Field[Any].

JelleZijlstra · 2018-03-29T02:02:20Z

stdlib/3.7/dataclasses.pyi

+
+class _InitVarMeta(type): ...
+
+def asdict(obj: Any, *, dict_factory: _DictType = ...) -> _DictType: ...


This is not quite correct; the argument type should be something like Type[_DictType].

Also, the range of acceptable type is a little wider than what's given here: in practice, dict_factory can be any callable accepting a list of (key, value) tuples. We could perhaps better type this as: def asdict(obj: Any, *, dict_factory: Callable[[List[Tuple[str, Any]], _T]) -> _T: ..., where _T = TypeVar('_T').

All this also goes for astuple.

This took me a couple of readings but I now understand and agree that using Callable is the most accurate approach.

JelleZijlstra · 2018-03-29T02:06:12Z

stdlib/3.7/dataclasses.pyi

+class InitVar(metaclass=_InitVarMeta): ...
+
+def make_dataclass(cls_name: str, fields: Iterable[Union[str, Tuple[str, type], Tuple[str, type, Field]]], *,
+    bases: Tuple[type, ...] = ..., namespace: Dict[str, Any] = ...,


namespace is Optional

glenjamin · 2018-04-02T21:38:58Z

Is there a good way to pull this definition into a python 3.6 project? Will the file need to be duplicated this repo into the stdlib and thirdparty sections?

gvanrossum · 2018-04-02T21:57:19Z

We can use symbolic links.

glenjamin · 2018-04-02T22:03:01Z

Having now played around with this definition a bit, am I right in thinking that mypy is going to need some sort of custom support to be able to correctly type dataclasses?

The problem I'm currently running into is that although the dataclass' properties are correctly specified, there is nothing in the decorator's type signature to indicate that an __init__ method has been generated, so the type checker falls back to object.__init__ which doesn't accept any arguments.

gwk · 2018-04-02T22:04:09Z

thanks to everyone for the reviews; I'll continue working whenever I find a free moment, hopefully next week. @glenjamin what do you mean about 3.6? Here is my attempt to backport dataclasses for use in both 3.6 and 3.7: https://github.com/gwk/pithy/tree/master/pithy/dataclasses. backport.py is copied from the cpython, and backport.pyi is the annotations file from this PR. __init__.py uses the backport only if the 3.7 std import fails. Is this more or less what you are imagining?

Regarding __init__, you are correct. See above for the referenced mypy issue.

glenjamin · 2018-04-02T22:11:23Z

@glenjamin what do you mean about 3.6?

I was only asking about the typeshed mechanims required to use the same pyi file for both without having to maintain two copies - the answer provided by Guido in #1944 (comment) seems obvious in retrospect.

JelleZijlstra · 2018-04-02T22:15:43Z

It's actually not that obvious: see our woes in #1175.

ilevkivskyi · 2018-05-28T02:12:53Z

@gwk Do you have time to finish this within next week (or preferably next few days)? If you don't, then I will need to continue your work myself (Python 3.7 release is coming very soon).

JelleZijlstra · 2018-05-28T05:44:32Z

@ilevkivskyi if you like, I can take over the implementation here if you do the review.

gwk · 2018-05-28T13:43:06Z

@JelleZijlstra @ilevkivskyi I'll try to update this now to address your comments.

ilevkivskyi · 2018-05-28T22:46:44Z

I'll try to update this now to address your comments.

Great! Just please don't rebase so that it will be easier to review, and don't forget to push your local changes so that we see the new commits :-)

…llables; add MISSING; field typed with overloads; namespace is optional.

gwk · 2018-05-29T02:59:47Z

@JelleZijlstra I have pushed the changes. @ilevkivskyi apologies in advance, I had previously rebased this branch against master and was not in a position to undo that operation. I hope it doesn't cause too much trouble for review. I did make the changes as a second commit so you will at least be able to see them separately!

JelleZijlstra · 2018-05-29T04:31:35Z

Thanks! Could you fix the CI errors?

JelleZijlstra · 2018-05-29T04:32:21Z

stdlib/3.7/dataclasses.pyi

+
+
+@overload # `default` and `default_factory` are optional and mutually exclusive.
+def field(*, default: _T = ..., default_factory: _MISSING_TYPE = ...,


All three overloads are actually the same unless I'm missing something.

Can you elaborate? The return type of the last one is Field[Any], while the first two are constrained.

JelleZijlstra · 2018-05-29T04:33:11Z

stdlib/3.7/dataclasses.pyi

+
+class _InitVarMeta(type): ...
+
+def asdict(obj: Any, *, dict_factory: _DictFactory = ...) -> _T: ...


Needs to be _DictFactory[_T]. Maybe just skip the type alias since it's only used once and write Callable... here directly.

JelleZijlstra · 2018-05-29T04:34:09Z

stdlib/3.7/dataclasses.pyi

+
+_T = TypeVar('_T')
+
+_DictFactory = Callable[List[Tuple[str, Any]], _T]


This is not quite right; you need one more set of [] around the first argument to Callable, because that's how you indicate the argument list.

bluetech · 2018-05-29T09:39:59Z

Is there any avenue in the future to make dataclasses.replace type-safe? e.g.

@dataclass
class C:
    field: int

c = C(field='field')
dataclasses.replace(c, doesnotexist=10)  # Should not type check.
dataclasses.replace(c, field='notint')  # Should not type check.

Asking as a heavy user of this function (or rather attr.evolve currently).

…tory.

ilevkivskyi · 2018-05-29T12:38:33Z

Is there any avenue in the future to make dataclasses.replace type-safe?

This is work for mypy plugin, the stub is only part of the (unfinished) picture.

…ield`.

gwk · 2018-05-29T13:16:42Z

@JelleZijlstra @ilevkivskyi I can't figure out how to fix the error regarding field overlapping overloads, even after removing the _MISSING_TYPE parameters from the overload signatures.

stdlib/3.7/dataclasses.pyi:37: error: Overloaded function signatures 1 and 3 overlap with incompatible return types
stdlib/3.7/dataclasses.pyi:42: error: Overloaded function signatures 2 and 3 overlap with incompatible return types

What is strange is that when I remove the additional optional parameters as shown below, it passes:

@overload
def field(*, default: _T) -> Field[_T]: ...

@overload
def field(*, default_factory: Callable[[], _T]) -> Field[_T]: ...

@overload
def field() -> Field[Any]: ...

This suggests to me that mypy has a bug, possibly around non-optional parameters following a *.

JelleZijlstra · 2018-05-29T15:22:28Z

That does look like a bug. Some of the work @Michael0x2a is currently doing on mypy may end up fixing this.

Sometimes mypy reports errors on the overload definition, but then behaves correctly when you use it. Can you try adding # type: ignores into the stub and seeing whether mypy has correct output for a few calls like field(default=3), field(default_factory=int), etc.?

ilevkivskyi · 2018-05-30T16:27:35Z

@JelleZijlstra Yes, Michael's PR fixed it. I restarted the build after mypy merge and it is all green now. Is anything left to do with this PR?

ilevkivskyi · 2018-05-30T16:28:42Z

stdlib/3.7/dataclasses.pyi

+
+class FrozenInstanceError(AttributeError): ...
+
+class InitVar(metaclass=_InitVarMeta): ...


Hm, I think this one should be Generic[_T].

I don't understand the usage of InitVar; I simply translated this from the implementation. Nor do I know quite what you are asking me to change the definition to.

@ilevkivskyi if you tell me what you want me to change it to I'll do so and we can close this out. Thanks!

I would write

class InitVar(Generic[_T]): ...

and remove _InitVarMeta completely because it is private.

…eta.

ilevkivskyi

LGTM! @JelleZijlstra do you have other comments?

JelleZijlstra reviewed Mar 6, 2018

View reviewed changes

JelleZijlstra mentioned this pull request Mar 18, 2018

Implement 3.7 features #1965

Closed

41 tasks

gwk force-pushed the dataclasses branch from 8941c4e to c946a38 Compare March 26, 2018 17:07

gwk mentioned this pull request Mar 26, 2018

Add support for dataclasses python/mypy#4792

Closed

gwk force-pushed the dataclasses branch 2 times, most recently from 2648516 to c031abd Compare March 26, 2018 20:20

sproshev reviewed Mar 28, 2018

View reviewed changes

gwk force-pushed the dataclasses branch from c031abd to 3df59cd Compare March 28, 2018 18:09

JelleZijlstra requested changes Mar 29, 2018

View reviewed changes

gwk force-pushed the dataclasses branch from 3df59cd to b26ca6d Compare April 15, 2018 14:34

JelleZijlstra mentioned this pull request May 14, 2018

add support for dataclasses python/mypy#5010

Merged

gwk added 2 commits May 28, 2018 22:56

Preliminary implementation of stdlib/3.7/dataclasses.pyi.

49986e2

dataclass.pyi improvements: _DictFactory and _TupleFactory are now ca…

5ecafd6

…llables; add MISSING; field typed with overloads; namespace is optional.

gwk force-pushed the dataclasses branch from fc0a3f9 to 5ecafd6 Compare May 29, 2018 02:57

JelleZijlstra requested changes May 29, 2018

View reviewed changes

gwk added 2 commits May 29, 2018 08:25

dataclass.pyi typos: missing List; additional brackets around callable.

4b4362e

dataclasses improvement: inline single use of _DictFactory, _TupleFac…

3ce43b5

…tory.

gwk added 2 commits May 29, 2018 08:44

dataclasses flake8 fix: two spaces before inline comment.

fe8fd7f

dataclasses: remove excessive default parameters from overloads of `f…

f78cd56

…ield`.

ilevkivskyi reviewed May 30, 2018

View reviewed changes

dataclasses improvement: InitVar is now Generic[_T]; remove _InitVarM…

971430a

…eta.

ilevkivskyi approved these changes May 30, 2018

View reviewed changes

JelleZijlstra approved these changes May 30, 2018

View reviewed changes

JelleZijlstra merged commit 66226ab into python:master May 30, 2018

gwk deleted the dataclasses branch May 30, 2018 20:38

yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this pull request Jan 23, 2019

Preliminary implementation of stdlib/3.7/dataclasses.pyi. (python#1944)

61e56c4


		class _InitVarMeta(type): ...

		def asdict(obj: Any, *, dict_factory: _DictType = ...) -> _DictType: ...



		@overload # `default` and `default_factory` are optional and mutually exclusive.
		def field(*, default: _T = ..., default_factory: _MISSING_TYPE = ...,


		_T = TypeVar('_T')

		_DictFactory = Callable[List[Tuple[str, Any]], _T]


		class FrozenInstanceError(AttributeError): ...

		class InitVar(metaclass=_InitVarMeta): ...

Preliminary implementation of stdlib/3.7/dataclasses.pyi. #1944

Preliminary implementation of stdlib/3.7/dataclasses.pyi. #1944

Conversation

gwk commented Mar 6, 2018

Choose a reason for hiding this comment

gwk commented Mar 26, 2018

JelleZijlstra commented Mar 26, 2018

gwk commented Mar 26, 2018 • edited Loading

JelleZijlstra commented Mar 26, 2018

gwk commented Mar 27, 2018

Choose a reason for hiding this comment

gwk Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JelleZijlstra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glenjamin commented Apr 2, 2018

gvanrossum commented Apr 2, 2018 via email

glenjamin commented Apr 2, 2018

gwk commented Apr 2, 2018 • edited Loading

glenjamin commented Apr 2, 2018 • edited Loading

JelleZijlstra commented Apr 2, 2018

ilevkivskyi commented May 28, 2018

JelleZijlstra commented May 28, 2018

gwk commented May 28, 2018

ilevkivskyi commented May 28, 2018

gwk commented May 29, 2018

JelleZijlstra commented May 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bluetech commented May 29, 2018

ilevkivskyi commented May 29, 2018

gwk commented May 29, 2018

JelleZijlstra commented May 29, 2018

ilevkivskyi commented May 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi left a comment

Choose a reason for hiding this comment

gwk commented Mar 26, 2018 •

edited

Loading

gwk Mar 28, 2018 •

edited

Loading

gwk commented Apr 2, 2018 •

edited

Loading

glenjamin commented Apr 2, 2018 •

edited

Loading