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

Define __prepare__() in with_metaclass() #178

Merged
merged 2 commits into from
Sep 17, 2017

Conversation

jmoldow
Copy link
Contributor

@jmoldow jmoldow commented Feb 27, 2017

Define __prepare__() in with_metaclass()'s temporary
metaclass, and make sure that it passes the correct bases to the
real metaclass's __prepare__().

The temporary metaclass previously didn't extend the
__prepare__() method, which meant that if the real metaclass
had a __prepare__(), it wouldn't get called correctly. This
could lead to bugs in Python 3 code.

The temporary metaclass's __prepare__() gets called with
bases=(temporary_class,). Since there was no proxy in the
middle, that was getting passed directly to the real metaclass's
__prepare__(). But then, if the real class's __prepare__()
method depended on the bases, the logic would be incorrect.

This was a problem in projects that use enum / enum34 and
try to use with_metaclass(EnumMeta). enum34.EnumMeta doesn't
define __prepare__(), since it is a Python 2 backport. Python
3's enum.EnumMeta does define __prepare__(), but originally
didn't depend at all on the bases. But starting in Python 3.6,
enum.EnumMeta.__prepare__() will raise TypeError if the
bases aren't valid for an enum subclass. Thus, a codebase that
was successfully using enum / enum34 and
with_metaclass(EnumMeta) could break on Python 3.6.

Define `__prepare__()` in `with_metaclass()`'s temporary
metaclass, and make sure that it passes the correct bases to the
real metaclass's `__prepare__()`.

The temporary metaclass previously didn't extend the
`__prepare__()` method, which meant that if the real metaclass
had a `__prepare__()`, it wouldn't get called correctly. This
could lead to bugs in Python 3 code.

The temporary metaclass's `__prepare__()` gets called with
```bases=(temporary_class,)```.  Since there was no proxy in the
middle, that was getting passed directly to the real metaclass's
`__prepare__()`. But then, if the real class's `__prepare__()`
method depended on the bases, the logic would be incorrect.

This was a problem in projects that use `enum` / `enum34` and
try to use `with_metaclass(EnumMeta)`. `enum34.EnumMeta` doesn't
define `__prepare__()`, since it is a Python 2 backport. Python
3's `enum.EnumMeta` does define `__prepare__()`, but originally
didn't depend at all on the bases. But starting in Python 3.6,
`enum.EnumMeta.__prepare__()` will raise `TypeError` if the
bases aren't valid for an enum subclass. Thus, a codebase that
was successfully using `enum` / `enum34` and
`with_metaclass(EnumMeta)` could break on Python 3.6.
@benjaminp benjaminp merged commit 96b9328 into benjaminp:master Sep 17, 2017
benjaminp added a commit that referenced this pull request Sep 17, 2017
@jmoldow jmoldow deleted the with_metaclass_prepare branch September 17, 2017 19:27
jmoldow added a commit to box/box-python-sdk that referenced this pull request Sep 18, 2017
Add a fallback for `six.raise_from`, which isn't available in
six 1.4.0. It isn't available until six 1.9.0. We could also
have raised the lower bound for the six requirement, but this is
an easy way to allow clients to keep using their existing
versions of six.

Fix support for the latest version of six, 1.11.0. That release
changed the temporary metaclass returned from
`with_metaclass()`, such that it directly inherits from `type`,
instead of inheriting from the target metaclass [1]. We depended
on this detail, and the change caused

.. code-block:: python

    TypeError('metaclass conflict: ...')

to be raised when defining a class with `with_metaclass()`. We
fix this by manually selecting the most derived metaclass, and
including it in our temporary metaclass.

Also, `__prepare__` is now defined on the temporary metaclass,
in six 1.11.0 [2]. This allows us to skip our own definition of that
method, when using six>=1.11.0.

Fixes #228.

Fixes #239.

[1] <benjaminp/six#191>
[2] <benjaminp/six#178>
jmoldow added a commit to box/box-python-sdk that referenced this pull request Sep 18, 2017
Add a fallback for `six.raise_from`, which isn't available in
six 1.4.0. It isn't available until six 1.9.0. We could also
have raised the lower bound for the six requirement, but this is
an easy way to allow clients to keep using their existing
versions of six.

Fix support for the latest version of six, 1.11.0. That release
changed the temporary metaclass returned from
`with_metaclass()`, such that it directly inherits from `type`,
instead of inheriting from the target metaclass [1]. We depended
on this detail, and the change caused

.. code-block:: python

    TypeError('metaclass conflict: ...')

to be raised when defining a class with `with_metaclass()`. We
fix this by manually selecting the most derived metaclass, and
including it in our temporary metaclass.

Also, `__prepare__` is now defined on the temporary metaclass,
in six 1.11.0 [2]. This allows us to skip our own definition of that
method, when using six>=1.11.0.

Fixes #228.

Fixes #239.

[1] <benjaminp/six#191>
[2] <benjaminp/six#178>
jmoldow added a commit to box/box-python-sdk that referenced this pull request Sep 18, 2017
Add a fallback for `six.raise_from`, which isn't available in
six 1.4.0. It isn't available until six 1.9.0. We could also
have raised the lower bound for the six requirement, but this is
an easy way to allow clients to keep using their existing
versions of six.

Fix support for the latest version of six, 1.11.0. That release
changed the temporary metaclass returned from
`with_metaclass()`, such that it directly inherits from `type`,
instead of inheriting from the target metaclass [1]. We depended
on this detail, and the change caused

.. code-block:: python

    TypeError('metaclass conflict: ...')

to be raised when defining a class with `with_metaclass()`. We
fix this by manually selecting the most derived metaclass, and
including it in our temporary metaclass.

Also, `__prepare__` is now defined on the temporary metaclass,
in six 1.11.0 [2]. This allows us to skip our own definition of that
method, when using six>=1.11.0.

Fixes #228.

Fixes #239.

[1] <benjaminp/six#191>
[2] <benjaminp/six#178>
jmoldow added a commit to box/box-python-sdk that referenced this pull request Sep 19, 2017
Update minimum required version of `six` to 1.9.0, so that
`six.raise_from` can be safely used by all clients.

Fix support for the latest version of six, 1.11.0. That release
changed the temporary metaclass returned from
`with_metaclass()`, such that it directly inherits from `type`,
instead of inheriting from the target metaclass [1]. We depended
on this detail, and the change caused

.. code-block:: python

    TypeError('metaclass conflict: ...')

to be raised when defining a class with `with_metaclass()`. We
fix this by manually selecting the most derived metaclass, and
including it in our temporary metaclass.

Also, `__prepare__` is now defined on the temporary metaclass,
in six 1.11.0 [2]. This allows us to skip our own definition of that
method, when using six>=1.11.0.

Fixes #228.

Fixes #239.

[1] <benjaminp/six#191>
[2] <benjaminp/six#178>
jmoldow added a commit to box/box-python-sdk that referenced this pull request Sep 19, 2017
Add a fallback for `six.raise_from`, which isn't available in
six 1.4.0. It isn't available until six 1.9.0. We could also
have raised the lower bound for the six requirement, but this is
an easy way to allow clients to keep using their existing
versions of six.

Fix support for the latest version of six, 1.11.0. That release
changed the temporary metaclass returned from
`with_metaclass()`, such that it directly inherits from `type`,
instead of inheriting from the target metaclass [1]. We depended
on this detail, and the change caused

.. code-block:: python

    TypeError('metaclass conflict: ...')

to be raised when defining a class with `with_metaclass()`. We
fix this by manually selecting the most derived metaclass, and
including it in our temporary metaclass.

Also, `__prepare__` is now defined on the temporary metaclass,
in six 1.11.0 [2]. This allows us to skip our own definition of that
method, when using six>=1.11.0.

Fixes #228.

Fixes #239.

[1] <benjaminp/six#191>
[2] <benjaminp/six#178>
@scoder
Copy link

scoder commented Oct 20, 2017

I think the __prepare__ method should only be included in Py3 selectively, not in Py2, to reduce the risk of incompatibilities. It has lead to user problems with Cython's own Py2/3 compatibility layer, for example:
cython/cython#1936
The problem is that it unconditionally calls the metaclass' __prepare__ method, which might simply not be implemented in Py2.

jeffwidman added a commit to dpkp/kafka-python that referenced this pull request Oct 22, 2018
Bump `six` to `1.11.0`. Most changes do not affect us, but it's good to
stay up to date. Also, we will likely start vendoring `enum34` in which
case benjaminp/six#178 is needed.

Note that this preserves the `kafka-python` customization from #979
which has been submitted upstream as benjaminp/six#176 but not yet merged.
jeffwidman added a commit to dpkp/kafka-python that referenced this pull request Oct 22, 2018
Bump `six` to `1.11.0`. Most changes do not affect us, but it's good to
stay up to date. Also, we will likely start vendoring `enum34` in which
case benjaminp/six#178 is needed.

Note that this preserves the `kafka-python` customization from #979
which has been submitted upstream as benjaminp/six#176 but not yet merged.
jeffwidman added a commit to dpkp/kafka-python that referenced this pull request Oct 22, 2018
Bump `six` to `1.11.0`. Most changes do not affect us, but it's good to
stay up to date. Also, we will likely start vendoring `enum34` in which
case benjaminp/six#178 is needed.

Note that this preserves the `kafka-python` customization from #979
which has been submitted upstream as benjaminp/six#176 but not yet merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants