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

Support six.add_metaclass #3842

Merged
merged 8 commits into from Aug 31, 2017

Conversation

Projects
None yet
2 participants
@elazarg
Contributor

elazarg commented Aug 18, 2017

Fix #3365

There are no special checks for improper use (similar to with_metaclass).
Consistency checks are handled like in any other metaclass.

@ilevkivskyi

This comment has been minimized.

Show comment
Hide comment
@ilevkivskyi

ilevkivskyi Aug 30, 2017

Collaborator

OK, this one can be now updated after #3848 is merged (plus the suggested tests).

Collaborator

ilevkivskyi commented Aug 30, 2017

OK, this one can be now updated after #3848 is merged (plus the suggested tests).

elazarg added some commits Aug 30, 2017

Merge remote-tracking branch 'upstream/master' into metaclass-getattr
Conflicts:
	mypy/fastparse.py
	mypy/semanal.py
	test-data/unit/semanal-abstractclasses.test
	test-data/unit/semanal-classes.test
@ilevkivskyi

Thanks, looks good! Just two small comments.

@@ -3539,25 +3569,50 @@ class ArcMeta(GenericMeta, DestroyableMeta):
pass
class Arc(six.with_metaclass(ArcMeta, Generic[T_co], Destroyable)):
pass
@six.add_metaclass(ArcMeta)
class Arc1(Generic[T_co], Destroyable):

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Aug 30, 2017

Collaborator

This actually fails at runtime because of a metaclass conflict. Is it easy to detect this? If this is non-trivial, then I think we should not include it in this PR.

@ilevkivskyi

ilevkivskyi Aug 30, 2017

Collaborator

This actually fails at runtime because of a metaclass conflict. Is it easy to detect this? If this is non-trivial, then I think we should not include it in this PR.

This comment has been minimized.

@elazarg

elazarg Aug 30, 2017

Contributor

I believe this is not caught because the base class Generic is removed earlier in the analysis. It doesn't seem obvious to me how this should be fixed.

@elazarg

elazarg Aug 30, 2017

Contributor

I believe this is not caught because the base class Generic is removed earlier in the analysis. It doesn't seem obvious to me how this should be fixed.

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Aug 31, 2017

Collaborator

OK, them it is not easy to support this.

@ilevkivskyi

ilevkivskyi Aug 31, 2017

Collaborator

OK, them it is not easy to support this.

Show outdated Hide outdated test-data/unit/check-classes.test

elazarg added some commits Aug 30, 2017

@ilevkivskyi

Thanks, looks good now!

@@ -3539,25 +3569,50 @@ class ArcMeta(GenericMeta, DestroyableMeta):
pass
class Arc(six.with_metaclass(ArcMeta, Generic[T_co], Destroyable)):
pass
@six.add_metaclass(ArcMeta)
class Arc1(Generic[T_co], Destroyable):

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Aug 31, 2017

Collaborator

OK, them it is not easy to support this.

@ilevkivskyi

ilevkivskyi Aug 31, 2017

Collaborator

OK, them it is not easy to support this.

@ilevkivskyi ilevkivskyi merged commit 3ef1e18 into python:master Aug 31, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment