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

Using new 'bool' format character #60203

Closed
serhiy-storchaka opened this issue Sep 21, 2012 · 12 comments
Closed

Using new 'bool' format character #60203

serhiy-storchaka opened this issue Sep 21, 2012 · 12 comments
Assignees
Labels
3.12 bugs and security fixes extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Sep 21, 2012

BPO 15999
Nosy @larryhastings, @Rosuav, @serhiy-storchaka
PRs
  • bpo-15999: Accept arbitrary values for boolean parameters. #15609
  • bpo-15999: Clean up of handling boolean arguments. #15610
  • bpo-15999: Always pass bool instead of int to socket.setblocking(). #15621
  • bpo-15999: Always pass bool instead of int to the expat parser. #15622
  • Files
  • use_bool_parsing.diff
  • bool_cleanup.patch
  • accept_bool.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 = None
    created_at = <Date 2012-09-21.15:47:09.273>
    labels = ['extension-modules', 'interpreter-core', 'type-feature']
    title = "Using new 'bool' format character"
    updated_at = <Date 2019-09-01.09:16:53.793>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2019-09-01.09:16:53.793>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules', 'Interpreter Core']
    creation = <Date 2012-09-21.15:47:09.273>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['27244', '40809', '40810']
    hgrepos = []
    issue_num = 15999
    keywords = ['patch']
    message_count = 7.0
    messages = ['170897', '170898', '252765', '253154', '350940', '350941', '350942']
    nosy_count = 3.0
    nosy_names = ['larry', 'Rosuav', 'serhiy.storchaka']
    pr_nums = ['15609', '15610', '15621', '15622']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue15999'
    versions = ['Python 3.4']

    Linked PRs

    @serhiy-storchaka
    Copy link
    Member Author

    bpo-14705 added 'bool' format character to PyArg_ParseTuple for using it for parsing multiple boolean arguments in os module. The proposed patch uses this format character for other boolean arguments. This makes the sources simpler, clearer and more reliable, improves error messages, gets rid of a few rare errors.

    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Sep 21, 2012
    @larryhastings
    Copy link
    Contributor

    Patch looks fine in principle, though I'd want someone to go over the individual cases to make sure they make sense.

    Also, I have something up my sleeve regarding argument parsing for 3.4, and if it gets accepted we might be touching all this code anyway. I mention this just to say--you may want to hold off on more patches like this for now.

    @serhiy-storchaka serhiy-storchaka self-assigned this Dec 29, 2012
    @Rosuav
    Copy link
    Contributor

    Rosuav commented Oct 11, 2015

    Has this been entirely superseded by Argument Clinic?

    @serhiy-storchaka
    Copy link
    Member Author

    No, it is orthogonal to Argument Clinic. Usually converting to Argument Clinic didn't change semantic. However these changes conflicted with proposed patch.

    Rebased patch is splitted on two patches.

    bool_cleanup.patch. It almost doesn't change the behavior. It uses the "p" format unit instead of manually called PyObject_IsTrue(), passes boolean value instead 0/1 integers to functions that needs boolean, and made some arguments ("flush" in print(), "strict", "sort_keys" and "skipkeys" in json module) to be converted to boolean only once.

    accept_bool.patch. It makes a number of functions to accept arbitrary objects in boolean context, not just False/True and ints representable as C int. But I prefer first to commit the patch for bpo-24037.

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset eb89746 by Serhiy Storchaka in branch 'master':
    bpo-15999: Always pass bool instead of int to the expat parser. (GH-15622)
    eb89746

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 5eca7f3 by Serhiy Storchaka in branch 'master':
    bpo-15999: Always pass bool instead of int to socket.setblocking(). (GH-15621)
    5eca7f3

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 1f21eaa by Serhiy Storchaka in branch 'master':
    bpo-15999: Clean up of handling boolean arguments. (GH-15610)
    1f21eaa

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @gpshead
    Copy link
    Member

    gpshead commented Dec 3, 2022

    #15609 was merged.

    I didn't try to figure out if there were any more of these left, that PR from @serhiy-storchaka took care of a large number of them.

    @gpshead
    Copy link
    Member

    gpshead commented Dec 3, 2022

    I'll close this issue as the end point isn't well defined with something we can easily check "[x] Complete" on.

    But it could be considered ongoing work to clean up any remaining. Feel free to re-open if anyone has identified more.

    Or just file new more specific issues for each area of the codebase still being overly pedantic about bool in C code.

    @gpshead gpshead closed this as completed Dec 3, 2022
    @gpshead
    Copy link
    Member

    gpshead commented Dec 4, 2022

    reopening to track the conversation on the merged PR around if we actually want it or not.

    @gpshead gpshead reopened this Dec 4, 2022
    @gpshead gpshead added the 3.12 bugs and security fixes label Dec 4, 2022
    @gpshead gpshead self-assigned this Dec 4, 2022
    serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 4, 2022
    In case if only True/False be supported as boolean arguments in future,
    we should continue to support 1/0 here.
    serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 4, 2022
    serhiy-storchaka added a commit that referenced this issue Dec 4, 2022
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 4, 2022
    …ythonGH-99983)
    
    Unless we explicitly test non-bool values.
    (cherry picked from commit 76f43fc)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Dec 4, 2022
    … tests (pythonGH-99983)
    
    Unless we explicitly test non-bool values..
    (cherry picked from commit 76f43fc)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    miss-islington added a commit that referenced this issue Dec 4, 2022
    )
    
    Unless we explicitly test non-bool values.
    (cherry picked from commit 76f43fc)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    serhiy-storchaka added a commit that referenced this issue Dec 4, 2022
    …GH-99983) (GH-99989)
    
    Unless we explicitly test non-bool values.
    (cherry picked from commit 76f43fc)
    @serhiy-storchaka
    Copy link
    Member Author

    Opened a discussion: https://discuss.python.org/t/boolean-arguments/21662

    Gregory, please join. Feel free to rewrite my post or add your explanations (you are better in this).

    ambv pushed a commit that referenced this issue Dec 5, 2022
    In case if only True/False be supported as boolean arguments in future,
    we should continue to support 1/0 here.
    ambv pushed a commit to ambv/cpython that referenced this issue Dec 5, 2022
    …-99982)
    
    In case if only True/False be supported as boolean arguments in future,
    we should continue to support 1/0 here.
    (cherry picked from commit 922a6cf)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    ambv pushed a commit to ambv/cpython that referenced this issue Dec 5, 2022
    …-99982)
    
    In case if only True/False be supported as boolean arguments in future,
    we should continue to support 1/0 here.
    (cherry picked from commit 922a6cf)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    ambv added a commit that referenced this issue Dec 5, 2022
    …0017)
    
    In case if only True/False be supported as boolean arguments in future,
    we should continue to support 1/0 here.
    (cherry picked from commit 922a6cf)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    ambv added a commit that referenced this issue Dec 5, 2022
    …0016)
    
    In case if only True/False be supported as boolean arguments in future,
    we should continue to support 1/0 here.
    (cherry picked from commit 922a6cf)
    
    Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
    @serhiy-storchaka
    Copy link
    Member Author

    Seems the decision was to simply accept this. So I close this issue.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.12 bugs and security fixes extension-modules C modules in the Modules dir interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants