int.from_bytes() is broken for subclasses

[serhiy-storchaka]

BPO	23640
Nosy	@warsaw, @rhettinger, @facundobatista, @mdickinson, @vstinner, @bitdancer, @skrah, @ethanfurman, @eltoder, @serhiy-storchaka
Files	issue23640.stoneleaf.01.patch 0001-int.from_bytes-calls-constructor-for-subclasses.patch 0002-int.from_bytes-calls-constructor-for-subclasses.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2016-05-12.07:49:10.222>
created_at = <Date 2015-03-11.13:34:36.466>
labels = ['interpreter-core', 'type-bug']
title = 'int.from_bytes() is broken for subclasses'
updated_at = <Date 2016-05-12.12:23:16.965>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2016-05-12.12:23:16.965>
actor = 'pitrou'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2016-05-12.07:49:10.222>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2015-03-11.13:34:36.466>
creator = 'serhiy.storchaka'
dependencies = []
files = ['38444', '38447', '40606']
hgrepos = []
issue_num = 23640
keywords = ['patch']
message_count = 24.0
messages = ['237864', '237881', '237887', '237888', '237891', '237896', '239619', '239622', '240053', '240065', '240085', '251750', '251751', '251833', '251863', '252091', '252092', '265104', '265334', '265352', '265355', '265372', '265373', '265397']
nosy_count = 16.0
nosy_names = ['barry', 'rhettinger', 'facundobatista', 'mark.dickinson', 'vstinner', 'Arfrever', 'r.david.murray', 'eli.bendersky', 'skrah', 'ethan.furman', 'python-dev', 'eltoder', 'serhiy.storchaka', 'bru', 'juggernaut', 'iyogeshjoshi']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23640'
versions = ['Python 3.5', 'Python 3.6']

[serhiy-storchaka]

Example:

>>> import socket
>>> x = socket.AddressFamily.from_bytes(b'\1', 'big')
>>> type(x)
<enum 'AddressFamily'>
>>> int(x)
1
>>> str(x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/enum.py", line 464, in __str__
    return "%s.%s" % (self.__class__.__name__, self._name_)
AttributeError: 'AddressFamily' object has no attribute '_name_'
>>> repr(x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/enum.py", line 461, in __repr__
    self.__class__.__name__, self._name_, self._value_)
AttributeError: 'AddressFamily' object has no attribute '_name_'

[ethanfurman]

The only solution that is coming to mind is to have EnumMeta go through the other base classes, wrap any classmethods it finds, and ensure that they return their appropriate type and not an Enum type.

Any other ideas?

[bru]

Hi,
I feel like this behaviour does not only affect IntEnum & related but anything that inherits from int.

Example:
>>> class foo(int):
...     def __init__(self, value, base=10):
...         if value == 2:
...             raise ValueError("not that!!")
...         super(foo, self).__init__(value, base=base)
...
>>> x = foo.from_bytes(b"\2", "big")
>>> x
2
>>> type(x)
foo

2 solutions come to mind:

• always return an int, and not the type. deleting Objects/longobjects.c:4845,4866 does the job.
• instantiate the required type with the value as the (sole?) argument, correctly initializing the object to be returned. In Serhyi's example this results in x == AddressFamily.AF_UNIX. the same lines are concerned.

[serhiy-storchaka]

Not all classmethods are constructors.

I see two solutions:

1. Override from_bytes() in IntEnum and every int subclass that needs this.

2. Change int.from_bytes() to call __new__.

[ethanfurman]

I think the classmethod-as-constructor behavior is correct, so it's up to IntEnum (or EnumMeta, or foo, or ...), to work around the issue.

[bru]

I took the liberty of putting together a small patch which makes from_bytes() call & return type(value_found) if type != long (Serhiy's & my 2nd solution).

Ethan: in that case, type.from_bytes() would always return an int, and not resort to build-a-pseudo-int, right? That way not overriding from_bytes() would not result in strange behaviour.

[ethanfurman]

'from_bytes' is a classmethod. As such, it should return the same type as the class it is called on. If that wasn't the intent it would be a staticmethod instead.

It is the responsibility of the subclass to override base class behavior, not the other way around.

[bru]

I'm not sure how you can have both, those two seem opposite to me:

• if 'from_bytes' returns the same type as the class it is called on then
  the instantiation of the result object should go through its constructor
  (patch proposed)
• if the subclass should override base class behaviour then there is no
  reason for Enum.from_bytes to return the same type as the class it is
  called on, and therefore it should be made a classmethod.

2015-03-30 18:02 GMT+02:00 Ethan Furman <report@bugs.python.org>:

Ethan Furman added the comment:

'from_bytes' is a classmethod. As such, it should return the same type as
the class it is called on. If that wasn't the intent it would be a
staticmethod instead.

It is the responsibility of the subclass to override base class behavior,
not the other way around.

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue23640\>

---

[ethanfurman]

With the patch:

  --> import enum
  --> class Huh(enum.IntEnum):
  ...   blah = 2
  ...
  --> Huh.blah.from_bytes(b'\04', 'big')
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    File "/home/ethan/source/python/issue23640/Lib/enum.py", line 222, in __call__
      return cls.__new__(cls, value)
    File "/home/ethan/source/python/issue23640/Lib/enum.py", line 457, in __new__
      raise ValueError("%r is not a valid %s" % (value, cls.__name__))
  ValueError: 4 is not a valid Huh

This is not the correct behavior. An IntEnum should act like an int, and in cases where it can't and still be an IntEnum, it becomes an int. But this behavior is Enum specific, and I would not expect other int subclasses to need or want that behavior.

Also, in cases where class methods are alternate constructors there is no requirement that they go through the main __new__/init constructors to do their job.

In other words, if IntEnum.from_bytes (which is inherited) is not behaving correctly, it is up to IntEnum to fix it -- it is not the job of int, and this is not a bug in int.

[pitrou]

The fact that derived_int.from_bytes() doesn't call the derived constructor clearly sounds like a bug to me, regardless of whether IntEnum also has its own bugs.

[serhiy-storchaka]

This bug allows to create new bool instances.

>>> false = bool.from_bytes(b'\0', 'big')
>>> true = bool.from_bytes(b'\1', 'big')
>>> bool(false)
False
>>> bool(true)
True
>>> false is False
False
>>> true is True
False
>>> false
False
>>> true
False