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

Refactor dict result handling in Tkinter #66422

Closed
serhiy-storchaka opened this issue Aug 19, 2014 · 9 comments
Closed

Refactor dict result handling in Tkinter #66422

serhiy-storchaka opened this issue Aug 19, 2014 · 9 comments
Assignees
Labels
topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

BPO 22226
Nosy @terryjreedy, @serhiy-storchaka
Files
  • tkinter_splitdict.diff
  • tkinter_splitdict_2.patch
  • tkinter_splitdict_3.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 2014-09-06.19:53:17.607>
    created_at = <Date 2014-08-19.06:16:13.969>
    labels = ['type-bug', 'expert-tkinter']
    title = 'Refactor dict result handling in Tkinter'
    updated_at = <Date 2014-09-06.19:53:17.606>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-09-06.19:53:17.606>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2014-09-06.19:53:17.607>
    closer = 'serhiy.storchaka'
    components = ['Tkinter']
    creation = <Date 2014-08-19.06:16:13.969>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['36411', '36425', '36558']
    hgrepos = []
    issue_num = 22226
    keywords = ['patch']
    message_count = 9.0
    messages = ['225515', '225594', '225597', '226406', '226447', '226474', '226500', '226508', '226509']
    nosy_count = 4.0
    nosy_names = ['terry.reedy', 'gpolo', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22226'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patch refactors code which converts the result of Tcl call to Python dict. Added new private function _splitdict() which does this in more robust manner.

    Also this patch fixes a bug in Treeview.heading(). The heading subcommand of ttk::treeview return options dict in which one key surprisingly was not '-' prefixed.

    % ttk::treeview t
    % .t heading #0
    -text {} -image {} -anchor center -command {} state {}

    Current code unconditionally cuts "s" from the "state" key.

    >>> from tkinter import ttk
    >>> t = ttk.Treeview()
    >>> t.heading('#0')
    {'anchor': 'center', 'tate': '', 'image': '', 'text': '', 'command': ''}

    @serhiy-storchaka serhiy-storchaka self-assigned this Aug 19, 2014
    @serhiy-storchaka serhiy-storchaka added topic-tkinter type-bug An unexpected behavior, bug, or error labels Aug 19, 2014
    @terryjreedy
    Copy link
    Member

    The patch expands and fixes buggy ttk._dict_from_tcltuple, renames and moves it to tkinter._splitdict, and replace 4 similar blocks of code in tkinter with calls to _splitdict. Review published. The only substantive comment is about keeping the test, in a new location. Aside from the comments, this is a nice code improvement.

    @serhiy-storchaka
    Copy link
    Member Author

    The patch expands and fixes buggy ttk._dict_from_tcltuple, renames and moves it to tkinter._splitdict

    There are differences between these functions. tkinter._splitdict calls splitlist on its argument, ttk._dict_from_tcltuple requires argument to be tuple/list. ttk._dict_from_tcltuple calls tclobjs_to_py on the result, tkinter._splitdict left it to the caller. Instead tkinter._splitdict allows you to specify custom function for dict values convertion.

    Updated patch addresses Terry's comments.

    Function name is matter of bakeshedding.

    @serhiy-storchaka
    Copy link
    Member Author

    If there are no objections I'll commit the patch soon.

    @terryjreedy
    Copy link
    Member

    I made two style comments, one an expansion of a previous comment. I believe all other previous comments were handled ok.

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you Terry. Fixed style in two methods, added a summary to Treeview.set() docstring and changed returns/sets to return/set. But I disagree with other suggestions. Return value of 3-arg call is insignificant implementation detail. In any case these changes are irrelevant to this issue.

    @terryjreedy
    Copy link
    Member

    looks good to commit.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 6, 2014

    New changeset 7b0fdc1e917a by Serhiy Storchaka in branch '2.7':
    Issue bpo-22226: Added private function _splitdict() in the Tkinter module.
    http://hg.python.org/cpython/rev/7b0fdc1e917a

    New changeset f89995a4ec11 by Serhiy Storchaka in branch '3.4':
    Issue bpo-22226: Added private function _splitdict() in the Tkinter module.
    http://hg.python.org/cpython/rev/f89995a4ec11

    New changeset 11cf18ec1900 by Serhiy Storchaka in branch 'default':
    Issue bpo-22226: Added private function _splitdict() in the Tkinter module.
    http://hg.python.org/cpython/rev/11cf18ec1900

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you Terry for your review.

    @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-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants