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

Test failures in Python 3.11.0b3 #326

Closed
major opened this issue Jun 16, 2022 · 7 comments · Fixed by #329
Closed

Test failures in Python 3.11.0b3 #326

major opened this issue Jun 16, 2022 · 7 comments · Fixed by #329
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@major
Copy link

major commented Jun 16, 2022

Environment details

  • Programming language: Python
  • OS: Fedora 37 (rawhide)
  • Language runtime version: Python 3.11.0b3
  • Package version: v1.20.6

Steps to reproduce

  1. Run pytest in Python 3.11.0b3

The first failure appears in test_total_ordering_w_other_enum_type:

____________________ test_total_ordering_w_other_enum_type _____________________
    def test_total_ordering_w_other_enum_type():
        to_compare = enums_test.OneEnum.SOME_VALUE
    
        for item in enums_test.OtherEnum:
            assert not to_compare == item
>           assert to_compare.SOME_VALUE != item
tests/test_enum_total_ordering.py:52: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <enum.property object at 0x3ffa350e550>
instance = <OneEnum.SOME_VALUE: 1>, ownerclass = <enum 'OneEnum'>
    def __get__(self, instance, ownerclass=None):
        if instance is None:
            try:
                return ownerclass._member_map_[self.name]
            except KeyError:
                raise AttributeError(
                        '%r has no attribute %r' % (ownerclass, self.name)
                        )
        else:
            if self.fget is None:
>               raise AttributeError(
                        '%r member has no attribute %r' % (ownerclass, self.name)
                        )
E               AttributeError: <enum 'OneEnum'> member has no attribute 'SOME_VALUE'
/usr/lib64/python3.11/enum.py:198: AttributeError

The second is in test_enum_alias_good:

_____________________________ test_enum_alias_good _____________________________
    def test_enum_alias_good():
        # Have to split good and bad enum alias into two tests so that the generated
        # file descriptor is properly created.
        # For the python runtime, aliases are allowed by default, but we want to
        # make sure that the options don't cause problems.
        # For the cpp runtime, we need to verify that we can in fact define aliases.
>       class GoodMessage(proto.Message):
tests/test_fields_enum.py:386: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/test_fields_enum.py:387: in GoodMessage
    class GoodEnum(proto.Enum):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
mcls = <class 'proto.enums.ProtoEnumMeta'>, name = 'GoodEnum'
bases = (<enum 'Enum'>,)
attrs = {'_generate_next_value_': <function Enum._generate_next_value_ at 0x3ffa5a8f560>, '__module__': 'test_fields_enum', '__qualname__': 'test_enum_alias_good.<locals>.GoodMessage.GoodEnum', 'UNKNOWN': 0, 'DEFAULT': 0}
    def __new__(mcls, name, bases, attrs):
        # Do not do any special behavior for `proto.Enum` itself.
        if bases[0] == enum.IntEnum:
            return super().__new__(mcls, name, bases, attrs)
    
        # Get the essential information about the proto package, and where
        # this component belongs within the file.
        package, marshal = _package_info.compile(name, attrs)
    
        # Determine the local path of this proto component within the file.
        local_path = tuple(attrs.get("__qualname__", name).split("."))
    
        # Sanity check: We get the wrong full name if a class is declared
        # inside a function local scope; correct this.
        if "<locals>" in local_path:
            ix = local_path.index("<locals>")
            local_path = local_path[: ix - 1] + local_path[ix + 1 :]
    
        # Determine the full name in protocol buffers.
        full_name = ".".join((package,) + local_path).lstrip(".")
        filename = _file_info._FileInfo.proto_file_name(
            attrs.get("__module__", name.lower())
        )
    
        # Retrieve any enum options.
        # We expect something that looks like an EnumOptions message,
        # either an actual instance or a dict-like representation.
        pb_options = "_pb_options"
        opts = attrs.pop(pb_options, {})
        # This is the only portable way to remove the _pb_options name
        # from the enum attrs.
        # In 3.7 onwards, we can define an _ignore_ attribute and do some
        # mucking around with that.
        if pb_options in attrs._member_names:
>           idx = attrs._member_names.index(pb_options)
E           AttributeError: 'dict' object has no attribute 'index'
../../BUILDROOT/python-proto-plus-1.20.6-1.fc37.noarch/usr/lib/python3.11/site-packages/proto/enums.py:61: AttributeError

For the second one, one of my coworkers found that _member_names is now a dict from a recent PR.

@major major added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jun 16, 2022
@hroncok
Copy link
Contributor

hroncok commented Jun 16, 2022

AttributeError: <enum 'OneEnum'> member has no attribute 'SOME_VALUE'

This is because enum members do no longer have attributes with all other enum members.

Python 3.10:

>>> from enum import Enum
>>> class Color(Enum):
...     RED = 1
...     GREEN = 2
...     BLUE = 3
... 
>>> Color.RED
<Color.RED: 1>
>>> Color.RED.RED
<Color.RED: 1>
>>> Color.RED.RED.RED
<Color.RED: 1>
>>> Color.RED.RED.RED.BLUE
<Color.BLUE: 3>
>>> Color.RED.RED.RED.BLUE.BLUE.GREEN.RED
<Color.RED: 1>

Python 3.11:

>>> from enum import Enum
>>> class Color(Enum):
...     RED = 1
...     GREEN = 2
...     BLUE = 3
... 
>>> Color.RED
<Color.RED: 1>
>>> Color.RED.RED
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.11/enum.py", line 198, in __get__
    raise AttributeError(
    ^^^^^^^^^^^^^^^^^^^^^
AttributeError: <enum 'Color'> member has no attribute 'RED'

That assert there IMHO makes no sense because to_compare.SOME_VALUE is to_compare and the same assertion passed in the above line.

@parthea
Copy link
Contributor

parthea commented Jun 28, 2022

Thanks for reporting this issue @major. I'd like to propose that we wait until the next release of python 3.11 or until python/cpython#93910 is resolved before patching this library.

@major
Copy link
Author

major commented Jun 28, 2022

@parthea Thanks for having a look. At the moment, it's holding up about ~ 20 Google Cloud SDK packages in Fedora rawhide (next release). We have a feature freeze for that one in August, so hopefully we can get this moving before then. 🤞🏻

Relevant Fedora bug: BZ 2099815

@hroncok
Copy link
Contributor

hroncok commented Jun 28, 2022

I suppose that if we know how to patch, we can apply the patch temporarily in Fedora.

@hroncok
Copy link
Contributor

hroncok commented Jun 28, 2022

This way, the tests pass with both Python 3.11.0b3 and Python 3.10.5.

diff --git a/proto/enums.py b/proto/enums.py
index 6f13d32..6207ad7 100644
--- a/proto/enums.py
+++ b/proto/enums.py
@@ -58,8 +58,11 @@ class ProtoEnumMeta(enum.EnumMeta):
         # In 3.7 onwards, we can define an _ignore_ attribute and do some
         # mucking around with that.
         if pb_options in attrs._member_names:
-            idx = attrs._member_names.index(pb_options)
-            attrs._member_names.pop(idx)
+            if isinstance(attrs._member_names, list):
+                idx = attrs._member_names.index(pb_options)
+                attrs._member_names.pop(idx)
+            else:  # Python 3.11.0b3
+                del attrs._member_names[pb_options]
 
         # Make the descriptor.
         enum_desc = descriptor_pb2.EnumDescriptorProto(
diff --git a/tests/test_enum_total_ordering.py b/tests/test_enum_total_ordering.py
index ad7a369..584a183 100644
--- a/tests/test_enum_total_ordering.py
+++ b/tests/test_enum_total_ordering.py
@@ -49,7 +49,11 @@ def test_total_ordering_w_other_enum_type():
 
     for item in enums_test.OtherEnum:
         assert not to_compare == item
-        assert to_compare.SOME_VALUE != item
+        assert type(to_compare).SOME_VALUE != item
+        try:
+            assert to_compare.SOME_VALUE != item
+        except AttributeError:  # Python 3.11.0b3
+            pass
         with pytest.raises(TypeError):
             assert not to_compare < item
         with pytest.raises(TypeError):

@parthea
Copy link
Contributor

parthea commented Jun 28, 2022

@hroncok , The patch looks great. Please could you open a PR?

hroncok added a commit to hroncok/proto-plus-python that referenced this issue Jun 28, 2022
There are two changes:

Changes in the actual code:

 - _member_names changed from a list to a dict in python/cpython#28907
    - we instance-check and remove by list-specific or dict-specific way

Change in the tests only:

 - accessing other enum members via instance attributes is no longer possible
    - we access them via the class instead
    - we leave the original test in a try-except block

Some of the Python enum changes might get reverted,
see python/cpython#93910
But the fix is backwards compatible.

Fixes googleapis#326
@hroncok
Copy link
Contributor

hroncok commented Jun 28, 2022

@hroncok , The patch looks great. Please could you open a PR?

I've opened #329

parthea added a commit that referenced this issue Jan 5, 2023
* Adjust to enum changes in Python 3.11.0b3

There are two changes:

Changes in the actual code:

 - _member_names changed from a list to a dict in python/cpython#28907
    - we instance-check and remove by list-specific or dict-specific way

Change in the tests only:

 - accessing other enum members via instance attributes is no longer possible
    - we access them via the class instead
    - we leave the original test in a try-except block

Some of the Python enum changes might get reverted,
see python/cpython#93910
But the fix is backwards compatible.

Fixes #326

* ci: unit test session with python 3.11.0-beta.3

* ci: add python v3.11.0-beta.3 to noxfile.py

* another attempt to get python 3.11.0b3 working in github actions

* ci: use python 3.8 for docs check

* ci: fix docs build

* fix ci

* mark python 3.11 tests as required

* add python 3.11 to setup.py

* fix docs build

* remove python 3.11 test for unitcpp

* remove python 3.11 test for unitcpp

* remove python 3.11 test for unitcpp

* attempt to fix exclude in github action

Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants