Classes lack __annotations__ (PEP-526)

[kazaamjt]

With python 3.7 dataclasses were introduced.
They work just fine in Cython except for their standard init function.

How to reproduce:
test.pyx

from dataclasses import dataclass

@dataclass
class TestClass:
	x: int
	y: int
	some_string: str

This file compiles to Cython code just fine.
However, when ran from the python it raises an error:
test2.py

import test

test_class = test.TestClass(1, 2, 'string')

This raises the following:

Traceback (most recent call last):
  File ".\test2.py", line 3, in <module>
    test_class = test.TestClass(1, 2, 'string')
TypeError: __init__() takes 1 positional argument but 4 were given

if you rename test.pyx to test.py and do not compile it, no such error occurs.

my current workaround:
setting each property after initialization separately:
test.pyx remains unchanged.
test.py

import test

test_class = test.TestClass()
test_class.x = 1
test_class.y = 2
test_class.some_string = 'string'

[scoder]

I don't think we currently create a __annotations__ for classes, although the syntax tree has the necessary information available. See the description in
https://www.python.org/dev/peps/pep-0526/#runtime-effects-of-type-annotations

PR welcome.

[teskje]

I'm also stumbling over this currently, especially Python 3.6-style NamedTuples not working.

I don't have any experience with how Cython works under the hood, but I'd be willing to give fixing this a try if someone could point me where to start. Is there some relevant documentation somewhere?

[amitripshtos]

Having similliar issue with dataclasses:

@dataclass
class Test:
    name: str

When using regular dataclass when I import the file with the dataclass, I get:

TypeError: 'name' is a field but has no type annotation

I guess that's because cython does not creation annotations for classes.

[scoder]

As always, the most important bit are tests. I would add new ones in a file tests/run/pyclass_annotations_pep526.py (with a # tag: pure36 to prevent Py<3.6 from running in uncompiled). Look at the other pyclass tests to see how it's done, we basically use doctests (which are executed in Python and not compiled, as opposed to the rest of the file).

I first thought that this could be implemented after executing the class body, but it turns out that PEP 526 wants the __annotations__ dict to be available before that, so creating the empty dict and assigning it in __Pyx_Py3MetaclassPrepare() would be the right way.

Then each NameNode inside of the class body (entry.is_pyclass_attr) which holds a self.annotation will have to generate C code (in generate_assignment_code()) to look up the __annotations__ dict in the class scope and assign its name to its annotation .py_result() on an assignment (or creation of the name). I don't actually know if that happens before or after the assignment to the name (which might fail, but could be shadowed by a try-except). Probably something to test in CPython. Thus: tests.

[scoder]

Also note that there are probably relevant tests for PEP-526 in CPython's own test suite, which could be reused.

[teskje]

I unfortunately didn't have much time these past few weeks to look at this. I've come up with a couple of test cases, however, that should get us pretty far on the way to full support of __annotations__. You can check them out here.

I have two main questions now:

1. Do we need full compliance with CPython or would you also consider merging something incomplete if it enables the (correct) use of dataclasses and NamedTuples? I think it should be pretty straightforward to add the annotations to the __annotations__ dict (passing test case Annotations above), which should be enough for the aforementioned use cases. I'm not sure how hard it would be to satisfy some of the other test cases though.
2. @scoder's explanation above was quite helpful (thanks!), but it seems that NameNode.generate_assignment_code is only invoked for actual assignments. So if we have a type annotated class variable that doesn't get a value assigned (i.e. just foo: int), we cannot add that to __annotations__ this way. Is there some function that's invoked for class variable declarations regardless of assignment?

[scoder]

You could handle 2) in ExprStatNode, which wraps non-statement expressions, such as arbitrarily sprinkled NameNodes.

Regarding compliance, I don't think there is all that much missing. I certainly wouldn't want a random hack that makes it mostly work but would have to be replaced completely when doing it right. I'd rather take at least some steps in the right direction.

[scoder]

Oh, and in fact, ExprStatNode marks its expression as unused, so that the non-assignment NameNode.generate_code() should be able to know that it only needs to store its annotation and be done. Although it could be that these nodes are discarded somewhere in the compiler pipeline. In that case, they should stay if they have an annotation to share.

[scoder]

Regarding namedtuples specifically, there is also #536 that would probably still prevent the usage even if this ticket was resolved. PRs welcome.

[GliderGeek]

enabling annotations on a class would be very beneficial for me:

• dataclasses
• pydantic classes (very similar, but available in python:3.6)
• (nice to have) namedtuple, but as said this would need more fixes.

@scoder : i would be willing to help, but find it difficult to assess if this is a reasonable goal for me. i have quite some python experience and a little bit of C++ experience. can you give any indication on the amount of work this might take? (days, weeks, months?). i understand that this is very difficult to say upfront.

maybe also i can contribute in another way by researching some specifics / adding preliminary tests?

[GliderGeek]

@ra-kete : you're link to tests no longer works. can you provide them again?

[scoder]

With PEP-563 in place, I think this now got much easier since annotations are now plain and simple strings. We might get away with building the dict in Python and adding it to the class dict as is, similar to what we do for function annotations:

cython/Cython/Compiler/ExprNodes.py

Lines 9371 to 9377 in bcae4d1

	if annotations:
	annotations_dict = DictNode(self.pos, key_value_pairs=[
	DictItemNode(
	pos, key=IdentifierStringNode(pos, value=name),
	value=value)
	for pos, name, value in annotations])
	self.annotations_dict = annotations_dict.analyse_types(env)

So, I think most of the work will be collecting (from CPython)/adapting/writing tests, then building the __annotations__ dict from whatever there is in the class body, and then possibly a little bit of generated C code in the Py3ClassNode to add it to the class's namespace mapping.

@GliderGeek want to give it a try?

[GliderGeek]

thanks for the quick reply. i will try to get some proper time to work on this.
in the meantime i have tried a little workaround:

class Annotated:
    x: int
    y: int 

    __annotations__ = {'x': int, 'y': int}

this seems to work properly. is this what you meant by "get away with building the dict in Python and adding it to the class dict "?

[scoder]

Sorry, with "in Python", I meant "in the Python code of the Cython compiler", i.e. in AST nodes (that was very unclear), as opposed to "in hand written C code". As done for functions.

But the end result will then be generated C code that implements pretty much what you spelled out above, yes. (Although PEP 563 would make it __annotations__ = {'x': 'int', 'y': 'int'}).

[teskje]

@ra-kete : you're link to tests no longer works. can you provide them again?

Sorry, I can't. Cleaned up my repos some time ago and forgot about this issue, so those tests are gone for good, unfortunately.

[seandstewart]

Popping in here - I've got a fork where I've attempted your recommendation, @scoder.

I'm very unclear as to exactly what kind of StatNode objects to expect in the class_def body, so this is my best guess:

https://github.com/seandstewart/cython/blob/master/Cython/Compiler/ExprNodes.py#L9060

I've also added a test here, based on your recommendation:

https://github.com/seandstewart/cython/blob/master/tests/run/pyclass_annotations_pep526.py

[scoder]

@seandstewart cool! I was thinking that the annotations should be collected by the nodes themselves, but looking through the constraints in PEP-526 again, it seems that even a simple solution like this might work. The test is ok as well, but should be extended to cover more declaration cases from PEP-526. Note that you don't strictly need a test function. You can use the docstring of the class as well as a doctest.

Here's what CPython uses:
https://github.com/python/cpython/blob/master/Lib/test/ann_module.py
https://github.com/python/cpython/blob/master/Lib/test/ann_module2.py
https://github.com/python/cpython/blob/master/Lib/test/ann_module3.py

Test for those are here:
https://github.com/python/cpython/blob/master/Lib/test/test_grammar.py
https://github.com/python/cpython/blob/57572b103ebd8732ac21922f0051a7f140d0e405/Lib/test/test_parser.py#L149

I've just updated the test_grammar.py copy that we use in Cython from Py3.9, but excluding the tests that use the external modules. While they could be integrated, I think it's better to create a separate test set from the annotation modules.

[seandstewart]

Hey @scoder - thanks for the pointers!

I've made some progress here. I've got the compiler actually extracting the correct node and building the list of annotations. However, I've hit a snag. When passing in the simple annotation.string, building the annotations_dict seems to work fine, however annotations_dict.py_result() ends up None. I tried passing the AnnotationNode directly to the dict instead (this feels more compliant, since the pure-python result will either be a string object or the class itself, depending upon how it's declared) but then annotations_dict.analyse_types(code) fails.

The relevant portions:

https://github.com/seandstewart/cython/blob/master/Cython/Compiler/ExprNodes.py#L9060-L9077
https://github.com/seandstewart/cython/blob/master/Cython/Compiler/ExprNodes.py#L9096-L9098

[scoder]

I think this would better fit into Nodes.PyClassDefNode instead of ExprNodes.Py3ClassNode.

The creation of the DictNode() should be done at the end of PyClassDefNode.analyse_declarations(), which analyses the declared names and variables in the class, long before the code generation happens. The result can be prepended (?) to the self.body.stats list (self.body is a StatListNode), as an assignment expressed in a SingleAssignmentNode (see the usages in Parsing.py), which should assign the DictNode result to an IdentifierStringNode for the __annotations__ key (in Python: __annotations__ = {…}). The rest should then happen more or less automatically from the syntax tree nodes.

I also think that it's probably a good idea to move the annotations over from their owning nodes into the DictNode values, i.e. set them to None, to prevent duplicate node references in the tree. I don't think they are used elsewhere.

This also seems worth a new method. :)

[seandstewart]

Great, yes, I think this is definitely the preferred approach, and will be in-line with CPython’s behavior.

One caveat - annotations with assignments should remain in the class body, whereas annotations without assignments can be safely removed, I think.

I’ll play with CPython’s AST and see how they handle this.

[scoder]

Oh, I just meant to set .annotation = None, not the whole statement node. The node can just stay there, it won't hurt.

[scoder]

@seandstewart I don't think there's a PR for this yet, right? Could you open one, so that we can move this forward?

[seandstewart]

Hey @scoder - apologies for the radio silence here, I haven't been able to work on side projects for a while. I'll try and put something together soon, I hadn't quite got the tests passing, IIRC.

[GliderGeek]

awesome news that this is implemented 🎉 thanks a lot