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

Implement PEP-560: core support for generic types #2753

Closed
verailina opened this issue Dec 10, 2018 · 19 comments
Closed

Implement PEP-560: core support for generic types #2753

verailina opened this issue Dec 10, 2018 · 19 comments

Comments

@verailina
Copy link

verailina commented Dec 10, 2018

Seems that Generic.__class_getitem__ can't be called automatically after cythonization.

Code sample example.py:

from typing import Generic, TypeVar

T = TypeVar("T")
print(Generic[T])

Cythonization:
cythonize -a -i example.py

The cythonized code fails with TypeError:

>>> import example
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "example.py", line 5, in init example
    print(Generic[T])
TypeError: 'type' object is not subscriptable

Cython 0.29.1
Python 3.7.1

@scoder
Copy link
Contributor

scoder commented Dec 29, 2018

PEP-560 is not currently implemented. Probably easy to do, but needs to be done.
https://www.python.org/dev/peps/pep-0560/

@scoder scoder changed the title typing.Generic[T] doesn't work after cythonization for python 3.7 Implement PEP-560 Dec 29, 2018
@scoder scoder added this to the 3.0 milestone Dec 29, 2018
@scoder
Copy link
Contributor

scoder commented Dec 29, 2018

This should be relatively easy.

  • copy over the tests from CPython
  • run the tests to see what C code gets generated currently
  • copy over parts of the implementation from CPython, drop them in the right places according to the C code generated for the tests (probably just the part from CPython's abstract.c)
  • adapt and optimise.

PR welcome.

@scoder scoder changed the title Implement PEP-560 Implement PEP-560: core support for generic types Jan 18, 2019
@msg555
Copy link
Contributor

msg555 commented Apr 8, 2020

For whatever it's worth I'm still running into apparently this issue with Cython 0.29.16/Python 3.7.7 running the same test as the OP.

@scoder
Copy link
Contributor

scoder commented Apr 9, 2020

See #2753 (comment) above. PR welcome.

@msg555
Copy link
Contributor

msg555 commented Apr 15, 2020

My feeling is that #3518 should address. Seems like just dropping back to PyObject_GetItem is sufficient when we're not dealing with a mapping or sequence object. Would definitely still need to write some tests

@nikhilmishra000
Copy link

nikhilmishra000 commented Apr 21, 2020

it seems like #3518 did not entirely fix this issue -- there will be an error if you try to inherit from the subscripted class.

For example, with the following in example.py, and building the same way as above:

from typing import Generic, TypeVar

T = TypeVar("T")

print(Generic[T])

class MyGeneric(Generic[T]):
  pass

will give:

typing.Generic[~T]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "example.py", line 7, in init example
    class MyGeneric(Generic[T]):
TypeError: __init__() takes 3 positional arguments but 4 were given

@msg555
Copy link
Contributor

msg555 commented Apr 21, 2020

Yes, we skipped MRO support for this PR. That still needs to happen and is also part of PEP 560.

I guess it should be a different issue although it's certainly related.

@scoder
Copy link
Contributor

scoder commented Apr 21, 2020

@nikhilmishra000 I created #3537 from your comment. PR welcome.

@gst
Copy link

gst commented Feb 10, 2021

I am having the OP problem as well.. is there any workaround / solution ?

Conditions: python3.8 + 0.29.21

@scoder
Copy link
Contributor

scoder commented Feb 11, 2021

@gst, sure, use Cython 3.0, where the fix was implemented.

@gst
Copy link

gst commented Feb 11, 2021

@scoder I get the same error with cython 3.0(a6) .. :\

@scoder does that imply that the prob/bug will not be fixed in 0.29 branch/side ?

What I dont get is that I do not have the prob with python3.6 .. :\

@gst
Copy link

gst commented Feb 11, 2021

to be noted that with python3.6 and cython 0.29.21 I do NOT have the problem..

@da-woods
Copy link
Contributor

da-woods commented Feb 11, 2021

Python 3.6 didn't use __class_getitem__ so that's an unsurprising result. It was introduced in Python 3.7 to simplify the implementation of Typing.

I believe this won't be fixed in Cython 0.29.x - it's considered a new feature, not a bug fix, and new features aren't being added to 0.29.x any more.

No idea why this isn't working in the current Cython 3.0 version. It is in there and it is tested so everything (except inheritance) should work.

@gst
Copy link

gst commented Feb 11, 2021

No idea why this isn't working in the current Cython 3.0 version.

should I open a new issue with my env. details ?

@da-woods
Copy link
Contributor

@gst Certainly "example.py" from the original post of this issue works for me on the latest master. You might be better off asking on the Cython-users mailing list (with enough detail to reproduce your setup) since I don't think this is really a Cython bug

@gst
Copy link

gst commented Feb 11, 2021

with latest master :

TypeError: __init__() takes 3 positional arguments but 4 were given

with the following example:

from typing import Generic, TypeVar

T = TypeVar('T')


class FitnessCalculator(Generic[T]):
    pass

@da-woods
Copy link
Contributor

Inheritance from generic types definitely doesn't work right now - #3537. I don't have a good workaround.

@da-woods
Copy link
Contributor

Inheritance from generic types definitely doesn't work right now - #3537. I don't have a good workaround.

OK - the workaround is to explicitly spell out all the steps that should be done automatically

from typing import Generic, TypeVar

T = TypeVar('T')

gt = Generic[T]

class FitnessCalculator(*gt.__mro_entries__((gt,))):
    __orig_bases__ = (gt,)
    pass

@gst
Copy link

gst commented Feb 11, 2021

I confirm the workaround works.. it's not very nice but it works. Thanks for it :)

aleksul added a commit to aleksul/pydantic that referenced this issue Jan 19, 2022
Watch cython issue cython/cython#2753
Previous implementation can be used after cython 3.0 release
samuelcolvin added a commit to pydantic/pydantic that referenced this issue May 14, 2022
…ints (#3681)

* Add tests

* Fix the issue

* Add changes file

* Improved convert_generics

* Add default fallback to convert_generics
Improved Annotated and Literal handling

* Fix Cython doesn't support generic types (PEP560)
Watch cython issue cython/cython#2753
Previous implementation can be used after cython 3.0 release

* Add custom type test

* Cosmetic fixes

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

* Fix typos

* Add SelfReferencing test validation
Add parametrization to

* Fix: parametrization caused test discovery problem

* Better explanation for a test case

* Better assertions for model creation tests

* Rerun CI

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>
ntaylorwss pushed a commit to nicejobinc/pydantic that referenced this issue Jun 24, 2022
…ints (pydantic#3681)

* Add tests

* Fix the issue

* Add changes file

* Improved convert_generics

* Add default fallback to convert_generics
Improved Annotated and Literal handling

* Fix Cython doesn't support generic types (PEP560)
Watch cython issue cython/cython#2753
Previous implementation can be used after cython 3.0 release

* Add custom type test

* Cosmetic fixes

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

* Fix typos

* Add SelfReferencing test validation
Add parametrization to

* Fix: parametrization caused test discovery problem

* Better explanation for a test case

* Better assertions for model creation tests

* Rerun CI

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants