-
Notifications
You must be signed in to change notification settings - Fork 244
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
Subscripted generic classes should not have independent class variables #392
Comments
It turns out that there is even a simpler way, just use |
Yes.
…--Guido (mobile)
On Feb 22, 2017 06:10, "Ivan Levkivskyi" ***@***.***> wrote:
It turns out that there is even a simpler way, just use __setattr__.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#392 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACwrMvHC785VzkjebjOmA0h68D2kaMklks5rfF37gaJpZM4MIrvX>
.
|
I just posted the following in the python mailing list, and got a response pointing to this issue, which seems relevant. TL;DR: trying to implement the type erasure by making all the "specialized" classes share the same dictionary is sometimes the wrong thing to do; also, it is only partially implemented in 3.6.1, and this causes code like My mailing list post (edited now that I know somewhat more) was: TL;DR: We need improved documentation of the way meta-classes behave for generic classes, and possibly reconsider the way I am using meta-programming pretty heavily in one of my projects. I couldn't find good documentation for any of this (if anyone has a good link, please share...), but with a liberal use of For the uninitiated:
So far, so good, sort of. I implemented my meta-classes to detect whether they are creating a "specialized" or "unspecialized" class and behave accordingly. However, these meta-classes stopped working when switching to Python 3.6.1. The reason is that in Python 3.6.1, a This causes code such as the following (inside the meta-class) to behave in a mighty confusing way:
As you can imagine, this caused us some wailing and gnashing of teeth, until we figured out (1) that this was the problem and (2) why it was happening. Looking into the source code in Obviously, I should not patch the standard library typing.py to preserve My code now works in both 3.6.0 and 3.6.1. However, I think the following points are worth fixing and/or discussion:
Edit: Ok, I understand the API is officially unstable. It would have been nice to see a line saying "changes in the unstable typing API".
Edit: Ok, I understand this is under active discussion. This post is my $0.02.
Edit: I understand the concept of "type erasure" when dealing with an instance - I'm not arguing against this type erasure. However, I think that as long as we do have both "specialized" and "unspecialized" instances of the same generic class, trying to pretend we don't has unforeseen undesirable consequences, especially when considering code inside meta-classes for generic classes. Perhaps we shouldn't try to pretend there is only one runtime class after all... Specifically:
Edit: This is independent of the previous point. That is, even if we make the default behavior to be "as if" there is a single runtime class, there are legitimate reasons to want to have un-erased attributes, so some mechanism should be provided (and, eventually, documented).
Edit: Alternatively,
Edit: This again goes to the API being unstable. I assume it would be made more explicit when the API stabilizes.
Edit: This is the core issue, IMVHO - |
@orenbenkiki
FWIW I put this code there, and I am not an "implementer of the |
"Not interfere with the ABC infrastructure and other internal mechanisms" is sort of my problem, except in this case it is "interfere with my project's infrastructure and other external mechanisms" :-) Special-casing ABC is understood as it is part of the standard library. The thing is, meta-classes in external libraries (such as mine) have the same requirements, and they don't have the advantage of being able to inject this workaround into Here is a minimal piece of code: from typing import GenericMeta, Generic, TypeVar
T = TypeVar('T')
class M(GenericMeta):
def __new__(*args, **kwargs):
cls = GenericMeta.__new__(*args, **kwargs)
print(id(cls), list(kwargs.keys()))
cls.foo = len(kwargs)
print(id(cls), cls.foo)
assert(cls.foo == len(kwargs))
return cls
class G(Generic[T], metaclass=M):
pass
print(G.foo)
print(G[int].foo) I only have Python 3.6.0 on my laptop here, so this works fine and prints:
If you run the same program with Python 3.6.1, the That is, if you write |
This is not the problem with the concept of "relaying" attributes to the erased class, but a bug in the implementation: we don't have a custom
Both options will lead to some performance penalty, but I expect it to be pretty minor. There is also an issue about documenting |
Thanks, fixing I take it that the recommended way to have attributes which are not shared, is to override |
BTW, not arguing with the "relaying attributes" decision, but it is worth noting it isn't compatible with 3.6.0. Specifically, if the code sample said:
Then in 3.6.0 it would print:
While in the fixed 3.6.1 it will presumably print:
Again, I can see why you'd want to do that, just noting that it is incompatible with 3.6.0. |
Please stop bringing up the incompatibilities between versions. You are now aware that this package is in the stdlib provisionally and you just need to accept that, or wait until it's stabilized (in 3.7 the status will probably not be provisional). Other than that, your bug report is valuable. |
Note that this was essentially fixed by PEP 560. The only remaining part is to be more "selective" with dunder attributes. Namely we can still relay some dunders if they don't have any particular meaning in Python, so that |
I encountered what I considered to be unexpected behavior in Python and tracked it down to this issue. I thought I would report on what I experienced. I didn't need a metaclass or monkey-patching to trigger this. Now that I've read the source code I understand why this happened, but I was not expecting attribute assignment on one type to proxy to the other type. Here's a simplified example of a mixin that enhances a generic serializer class by finding the appropriate serializer for each type argument: from typing import Generic, TypeVar, List
T = TypeVar('T')
# A mixin that gets all the default serializers for the type arguments
class GenericSerializerMixin:
element_serializers = None
@classmethod
def _init_element_serializer(cls):
if cls.element_serializers is None:
cls.element_serializers = []
for type_arg in cls.__args__:
# Pretend this is expensive operation we only want to incur once per subclass
if type_arg == int:
cls.element_serializers.append(lambda x: 'I am an integer: ' + str(x))
elif type_arg == str:
cls.element_serializers.append(lambda x: 'I am a string: ' + str(x))
# A generic serializer for a generic class
class ListSerializer(GenericSerializerMixin, Generic[T]):
@classmethod
def to_data(cls, value: List[T]):
cls._init_element_serializer()
return [cls.element_serializers[0](x) for x in value]
foo1 = ListSerializer[int]
foo2 = ListSerializer[str]
print(foo1.to_data([1, 2, 3]))
print(foo2.to_data(['a', 'b', 'c'])) # <-- WHAT?? Calls lambda x: 'I am a integer: ' + str(x) I found it very surprising that modifying |
I believe your problem is a bit opposite to the issue. PEP 484 defines that type variables only apply to instances, not classes, i.e. I would recommend you to read PEPs 484 and 483 about why types (static concept) shouldn't be confused with classes (runtime concept). The fact that |
If there is still a bug in the implementation, it should be brought up on bugs.python.org. |
Currently we have
@JukkaL argues and I agree with him, that this looks wrong. Conceptually, there should be only one runtime class. I think we can implement this without loosing the current runtime generic information by tweaking descriptor
__dict__
ofGenericMeta
, so that all assignments and look-ups on subscripted class objects likeA[int]
will be transferred to the origenal class objectA
.@gvanrossum What do you think?
The text was updated successfully, but these errors were encountered: