Allow docstrings in Python 3.6+ class based TypedDict

[JakobGM]

At the moment, if you try to define the following type,

from mypy_extensions import TypedDict

class GroupJSON(TypedDict):
    """ JSON representation of group, received from API-endpoint """
    group_name: str

you end up receiving an mypy error saying something along the lines of expected format 'key_name: value_type'. Excuse me for not providing the exact error message, as I won't be able to use a computer for a couple of days.

It can be avoided by using the normal #-comment syntax. It still came as a surprise when using this syntax. Most people are probably used to documenting classes with docstrings.

Perhaps this behaviour is intentional, as TypedDict instances are interpreted as normal dictionaries at runtime? I would still argue that this is an implementation detail leaking through to the interface. Perhaps the docstring can be entirely ignored at runtime?

Thanks in advance,
Jakob

[ilevkivskyi]

This is a reasonable suggestion, and should be easy to fix. Would you like to make a PR at some point?

[JakobGM]

@ilevkivskyi I will definitely try!

Will read through the source code the next couple of days, and will see if I am able to implement it when I'm able to use the computer again.

[JakobGM]

Seems like it can be fixed by adding an extra conditional in this branch. Now I only need some sort of DocstrExpr or TripleQuoteStringLiteralExpr from mypy.nodes. Investigating this is next on the agenda.

[JelleZijlstra]

FYI, I implemented docstrings on NamedTuple classes a while ago, which should be very similar. This was done in #3081; the commit that added support for doc strings was 68d16c0.

[JakobGM]

@JelleZijlstra Great, that will help a lot. Thanks!

[miedzinski]

diff --git a/mypy/semanal.py b/mypy/semanal.py
index 4f0c9793..e0206aa9 100644
--- a/mypy/semanal.py
+++ b/mypy/semanal.py
@@ -1280,7 +1280,7 @@ class SemanticAnalyzer(NodeVisitor[None]):
                 # Still allow pass or ... (for empty TypedDict's).
                 if (not isinstance(stmt, PassStmt) and
                     not (isinstance(stmt, ExpressionStmt) and
-                         isinstance(stmt.expr, EllipsisExpr))):
+                         isinstance(stmt.expr, (EllipsisExpr, StrExpr)))):
                     self.fail(TPDICT_CLASS_ERROR, stmt)
             elif len(stmt.lvalues) > 1 or not isinstance(stmt.lvalues[0], NameExpr):
                 # An assignment, but an invalid one.

This is sufficient to make it work, but will still reject (similar to namedtuple)

class A(mypy_extensions.TypedDict):
    __doc__ = "foo"

I had a patch with a test ready an hour ago, but had to leave house and couldn't respond here in time. :) If it's enough I can send a PR.

[JakobGM]

@miedzinski Looks good!

In the case of namedtuple, it makes sense that you would not be able to add a __doc__ type as that would entail the use of named_tuple_instance.__doc__ as anything other than a str, which is highly unconventional.

Well, that is not necessarily the case for TypedDict, though, is it? I know it is an edge case, but should mypy support dictionary keys named "__doc__"?

[miedzinski]

IMO it shouldn't. The thing is

class A(TypedDict):
    __doc__ = "foo"

should be equivalent to

class A(TypedDict):
    """foo"""

but the former fails with error: Invalid statement in TypedDict definition; expected "field_name: field_type".

I can submit a PR with the patch above and if anyone ever needs to support __doc__ = "..." then we can rethink what to do next. It is doable, but check_typeddict_classdef doesn't take TypeInfo as parameter so it requires a bigger patch with some refactoring to make it work. :)

@ilevkivskyi is it okay?

[ilevkivskyi]

is it okay?

Yes