classification
Title: Consider isinstance(..., numbers.Integral) instead of isinstance(..., int) or isinstance(..., (int, long)) in datetime.py
Type: behavior Stage: needs patch
Components: Library (Lib) Versions: Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, posita, serhiy.storchaka, tim.peters
Priority: normal Keywords:

Created on 2015-11-23 18:42 by posita, last changed 2015-12-01 22:39 by posita.

Messages (3)
msg255211 - (view) Author: Matt Bogosian (posita) * Date: 2015-11-23 18:42
datetime.py is peppered with ``assert`` statements that are two restrictive. Specifically, ``isinstance(..., int)`` does not afford compatibility with alternate ``int``-like implementations (e.g., ``future.types.newint``). Some background:

* https://github.com/PythonCharmers/python-future/issues/187
* https://bitbucket.org/pypy/pypy/issues/2193/datetimetimedelta-chokes-on-seconds

In all cases that I can find, ``assert isinstance(..., int)`` is unnecessarily restrictive, since it is always followed by another ``assert`` which validates that the boundaries are far smaller than a Python 2 native `int` type. (See, e.g., `datetype.py at line 395 <https://hg.python.org/cpython/file/ebec1a98ab81/Lib/datetime.py#l395>`__).

I propose replacing instances of ``assert isinstance(..., int)`` and ``assert isinstance(..., (int, long))`` with ``assert isinstance(..., numbers.Integral)`` akin to `this patch <https://bitbucket.org/pypy/pypy/pull-requests/361/fix-2193/diff>`__.
msg255267 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-24 14:54
C implementation almost everywhere (except strftime/strptime) accepts long as well as int, but not general integral numbers. Python implementation of the datetime module is not entire compatible with C implementation (default in CPython), therefore alternative Python implementations that doesn't use an accelerator is incompatible with CPython. I consider this as a bug and think that we should weak some of checks isinstance(..., int) to isinstance(..., (int, long)). numbers.Integral is too wide type, C implementation doesn't support it.
msg255681 - (view) Author: Matt Bogosian (posita) * Date: 2015-12-01 22:39
> I consider this as a bug and think that we should weak some of checks isinstance(..., int) to isinstance(..., (int, long)). numbers.Integral is too wide type, C implementation doesn't support it.

Oddly enough, the C implementation is what is working with `future` in this case (although I take your point regarding `numbers.Integral` being too wide; `future.types.newint` effectively behaves like a `long`, and `isinstance(future.types.newint(1), long)` returns `True`).
History
Date User Action Args
2015-12-01 22:39:16positasetmessages: + msg255681
2015-11-24 14:54:55serhiy.storchakasetnosy: + serhiy.storchaka, belopolsky
messages: + msg255267

type: enhancement -> behavior
stage: needs patch
2015-11-24 08:54:50rhettingersetnosy: + tim.peters
2015-11-23 18:42:19positacreate