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

Tkinter doesn't support large integers #61044

Closed
serhiy-storchaka opened this issue Jan 2, 2013 · 21 comments
Closed

Tkinter doesn't support large integers #61044

serhiy-storchaka opened this issue Jan 2, 2013 · 21 comments
Assignees
Labels
topic-tkinter type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 16840
Nosy @loewis, @vstinner, @benjaminp, @serhiy-storchaka
Files
  • tkinter_bignum_4.patch
  • tkinter_hexversion.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 2015-04-22.12:33:08.917>
    created_at = <Date 2013-01-02.15:09:13.577>
    labels = ['type-feature', 'expert-tkinter']
    title = "Tkinter doesn't support large integers"
    updated_at = <Date 2015-04-22.12:33:08.916>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2015-04-22.12:33:08.916>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-04-22.12:33:08.917>
    closer = 'serhiy.storchaka'
    components = ['Tkinter']
    creation = <Date 2013-01-02.15:09:13.577>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['38198', '39158']
    hgrepos = []
    issue_num = 16840
    keywords = ['patch', 'needs review']
    message_count = 21.0
    messages = ['178815', '178988', '179204', '195916', '218743', '235098', '236375', '236381', '236386', '236387', '239928', '239932', '240224', '240228', '240442', '241628', '241630', '241706', '241708', '241729', '241785']
    nosy_count = 6.0
    nosy_names = ['loewis', 'vstinner', 'benjamin.peterson', 'gpolo', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16840'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @serhiy-storchaka
    Copy link
    Member Author

    A long time Tcl got support first 64-bit integers, and then arbitrary size integers. Python also supports arbitrary size integers. However Tkinter supports only C long integers. For example, on 32-bit platform:

    >>> import tkinter
    >>> t = tkinter.Tk()
    >>> t.tk.call('expr', 2**30)
    1073741824
    >>> t.tk.call('expr', 2**31)
    <wideInt object at 0x9122518>
    >>> t.tk.call('expr', 2**63)
    <bignum object at 0x9126010>

    Those <wideInt object> and <bignum object> are not usable from Python. Potentially this can cause errors in some rare circumstances.

    I'm working on a patch, but was faced with a problem. Tcl provides functions for conversions between Bignum Tcl object and mp_int, but it does not provide all functions for conversions between mp_int and bytes sequence. Which is better, add libtommath library to CPython dependencies, or implement the missing functionality manually? Or may be use haxadecimal representation for conversions?

    @serhiy-storchaka serhiy-storchaka self-assigned this Jan 2, 2013
    @serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error topic-tkinter labels Jan 2, 2013
    @serhiy-storchaka
    Copy link
    Member Author

    Here is a patch which adds support of "wideInt" and "bignum" Tcl types.

    @serhiy-storchaka
    Copy link
    Member Author

    I reclassify this as enhancement until the real bug is not found.

    @serhiy-storchaka serhiy-storchaka added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jan 6, 2013
    @serhiy-storchaka
    Copy link
    Member Author

    Patch updated.

    @serhiy-storchaka
    Copy link
    Member Author

    Patch synchronized with tip and fixed support of Tcl <8.5.

    @serhiy-storchaka
    Copy link
    Member Author

    Here is test failure related to this issue:

    http://buildbot.python.org/all/builders/AMD64%20Windows8%202.7/builds/156/steps/test/logs/stdio

    test test_tk failed -- Traceback (most recent call last):
      File "D:\buildarea\2.7.bolen-windows8\build\lib\lib-tk\test\test_tkinter\test_widgets.py", line 91, in test_use
        wid = parent.winfo_id()
      File "D:\buildarea\2.7.bolen-windows8\build\lib\lib-tk\Tkinter.py", line 839, in winfo_id
        self.tk.call('winfo', 'id', self._w))
    TclError: integer value too large to represent

    @serhiy-storchaka
    Copy link
    Member Author

    Updated patch now supports wideInt if PY_LONG_LONG is not defined or is not equal to Tcl_WideInt. getint() also now supports big integers (currently it returns ambiguous result for values outside the range of C signed int).

    As far as this issue causes random tests failures, may be it makes sense to apply the patch to maintained releases. On other hand, looks as this happens very rarely.

    @benjaminp
    Copy link
    Contributor

    I'm all for fixing random test failures.

    @serhiy-storchaka
    Copy link
    Member Author

    But there is a risk to break Python builds with old Tcl/Tk. We dropped support of Tcl/Tk older than 8.4 in 3.5, but theoretically 2.7 should work with older versions. It was not tested for years, we have no build bots with 8.3, and I manually test Python only with 8.4+ (it is harder to build 8.3, not mentioning older versions). But AFAIK wideInt was not supported in 8.3, so code and tests should be adapted for optional support of integers outside 32-bit range.

    I could try to build 8.3 and adapt the patch, but I'm not very motivated in this. And I can't take the responsibility of committing the patch blindly, without testing with different options.

    @benjaminp
    Copy link
    Contributor

    It's up to you. Since 2.7 is so long lived, we've had to gradually alter supported library versions (e.g. for bsddb).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 2, 2015

    New changeset 42a6449e577c by Serhiy Storchaka in branch '2.7':
    Issue bpo-16840: Tkinter now supports 64-bit integers added in Tcl 8.4 and
    https://hg.python.org/cpython/rev/42a6449e577c

    New changeset a6f4d8fa7ab8 by Serhiy Storchaka in branch '3.4':
    Issue bpo-16840: Tkinter now supports 64-bit integers added in Tcl 8.4 and
    https://hg.python.org/cpython/rev/a6f4d8fa7ab8

    New changeset 9291b28157e1 by Serhiy Storchaka in branch 'default':
    Issue bpo-16840: Tkinter now supports 64-bit integers added in Tcl 8.4 and
    https://hg.python.org/cpython/rev/9291b28157e1

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 2, 2015

    New changeset 9905fb0b5885 by Serhiy Storchaka in branch '2.7':
    Issue bpo-16840: Fixed test_tcl for Tcl < 8.5.
    https://hg.python.org/cpython/rev/9905fb0b5885

    New changeset 1d2444273b3d by Serhiy Storchaka in branch '3.4':
    Issue bpo-16840: Fixed test_tcl for Tcl < 8.5.
    https://hg.python.org/cpython/rev/1d2444273b3d

    New changeset 2398dba039f3 by Serhiy Storchaka in branch 'default':
    Issue bpo-16840: Fixed test_tcl for Tcl < 8.5.
    https://hg.python.org/cpython/rev/2398dba039f3

    @vstinner
    Copy link
    Member

    vstinner commented Apr 7, 2015

    test_expr_bignum() should tolerate the long type:

    http://buildbot.python.org/all/builders/x86%20Tiger%202.7/builds/3007/steps/test/logs/stdio

    ======================================================================
    FAIL: test_expr_bignum (test.test_tcl.TclTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/db3l/buildarea/2.7.bolen-tiger/build/Lib/test/test_tcl.py", line 443, in test_expr_bignum
        self.assertIsInstance(result, type(int(result)))
    AssertionError: -2147483648L is not an instance of <type 'int'>

    In fact, I don't understand the test self.assertIsInstance(result, type(int(result))). What do you expect from type(int(x)): int or long depending on the value of x? The problem is that an intermediate result may be a long and so tcl.call('expr', ..) returns a long even if it may fit into a small int.

    I suggest to simply drop the test self.assertIsInstance(result, type(int(result))).

    @vstinner vstinner reopened this Apr 7, 2015
    @serhiy-storchaka
    Copy link
    Member Author

    It is important that the result is an int at least for small ints. So I prefer to keep limited test.

    •        self.assertIsInstance(result, type(int(result)))
      

    + if abs(result) < 2**31:
    + self.assertIsInstance(result, int)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 10, 2015

    New changeset 350c78a92046 by Serhiy Storchaka in branch '2.7':
    Issue bpo-16840: Fixed Tcl test on 2.7 with Tcl 8.4.19.
    https://hg.python.org/cpython/rev/350c78a92046

    @serhiy-storchaka
    Copy link
    Member Author

    There is at least one buildbot that now fails to build _tkinter.

    http://buildbot.python.org/all/builders/x86%20FreeBSD%206.4%202.7/builds/2457/steps/test/logs/stdio
    building '_tkinter' extension
    gcc -pthread -fPIC -fno-strict-aliasing -g -O2 -g -O0 -Wall -Wstrict-prototypes -DWITH_APPINIT=1 -I/usr/local/include/tcl8.5 -I/usr/local/include/tk8.5 -I/usr/X11R6/include -I. -IInclude -I./Include -I/usr/local/include -I/usr/home/db3l/buildarea/2.7.bolen-freebsd/build/Include -I/usr/home/db3l/buildarea/2.7.bolen-freebsd/build -c /usr/home/db3l/buildarea/2.7.bolen-freebsd/build/Modules/_tkinter.c -o build/temp.freebsd-6.4-RELEASE-i386-2.7-pydebug/usr/home/db3l/buildarea/2.7.bolen-freebsd/build/Modules/_tkinter.o
    /usr/home/db3l/buildarea/2.7.bolen-freebsd/build/Modules/_tkinter.c:101:24: tclTomMath.h: No such file or directory

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 20, 2015

    New changeset 8ab077c22fbf by Serhiy Storchaka in branch '2.7':
    Issue bpo-16840: Turn on support of bignums only in final release of Tcl 8.5.
    https://hg.python.org/cpython/rev/8ab077c22fbf

    New changeset 7f1622478d17 by Serhiy Storchaka in branch '3.4':
    Issue bpo-16840: Turn on support of bignums only in final release of Tcl 8.5.
    https://hg.python.org/cpython/rev/7f1622478d17

    New changeset 7a7f09528866 by Serhiy Storchaka in branch 'default':
    Issue bpo-16840: Turn on support of bignums only in final release of Tcl 8.5.
    https://hg.python.org/cpython/rev/7a7f09528866

    @serhiy-storchaka
    Copy link
    Member Author

    This didn't help. Actually we should turn on bignum support only since 8.5.8, because tclTomMath.h was broken before. But this is not related, because tclTomMath.h is not found itself on this buildbot.

    @serhiy-storchaka
    Copy link
    Member Author

    Ah, we should also test full version of 8.6, because bignums was not supported in earlier alphas and betas (if I remember correctly, this buildbot uses beta version of 8.6).

    TK_VERSION_HEX packs fields in wrong order, not suitable for comparing of non-final releases. It's value for 8.6a3 is larger than 8.6.1.

    Here is a patch that adds TK_HEX_VERSION with correctly packed fields and turn off bignum support in non-final 8.6.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 21, 2015

    New changeset 5116916e4f7f by Serhiy Storchaka in branch '2.7':
    Issue bpo-16840. Turn off bignum support in tkinter with with Tcl earlier than 8.5.8
    https://hg.python.org/cpython/rev/5116916e4f7f

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 22, 2015

    New changeset 4bf210f59ac6 by Serhiy Storchaka in branch '2.7':
    Issue bpo-16840: Skip bignum tests on minor releases where they are not supported.
    https://hg.python.org/cpython/rev/4bf210f59ac6

    New changeset 97519d85b5c8 by Serhiy Storchaka in branch '3.4':
    Issue bpo-16840. Turn off bignum support in tkinter with with Tcl earlier than 8.5.8
    https://hg.python.org/cpython/rev/97519d85b5c8

    New changeset 701b830abbf0 by Serhiy Storchaka in branch 'default':
    Issue bpo-16840. Turn off bignum support in tkinter with with Tcl earlier than 8.5.8
    https://hg.python.org/cpython/rev/701b830abbf0

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-tkinter type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants