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

Argument Clinic: add the boolint converter #68225

Closed
serhiy-storchaka opened this issue Apr 23, 2015 · 9 comments
Closed

Argument Clinic: add the boolint converter #68225

serhiy-storchaka opened this issue Apr 23, 2015 · 9 comments
Labels
3.7 (EOL) end of life topic-argument-clinic type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 24037
Nosy @taleinat, @larryhastings, @serhiy-storchaka
PRs
  • bpo-24037: Add Argument Clinic converter bool(accept={int}). #485
  • Files
  • clinic_boolint_converter.patch
  • clinic_boolint_converter_2.patch
  • clinic-boolint-converter-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 = None
    closed_at = <Date 2017-03-17.21:18:08.165>
    created_at = <Date 2015-04-23.10:33:08.833>
    labels = ['type-feature', '3.7', 'expert-argument-clinic']
    title = 'Argument Clinic: add the boolint converter'
    updated_at = <Date 2017-03-24.22:25:30.470>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-03-24.22:25:30.470>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-03-17.21:18:08.165>
    closer = 'serhiy.storchaka'
    components = ['Argument Clinic']
    creation = <Date 2015-04-23.10:33:08.833>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['39183', '39349', '46700']
    hgrepos = []
    issue_num = 24037
    keywords = ['patch']
    message_count = 9.0
    messages = ['241860', '241882', '241883', '242008', '242010', '242140', '242957', '288993', '290218']
    nosy_count = 3.0
    nosy_names = ['taleinat', 'larry', 'serhiy.storchaka']
    pr_nums = ['485']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue24037'
    versions = ['Python 3.7']

    @serhiy-storchaka
    Copy link
    Member Author

    The 'i' format unit is often used for semantically boolean values. When the parameter has a default value, in Argument clinic you should specify it twice, as Python and C values:

    closefd: int(c_default="1") = True
    

    or

    keepends: int(py_default="False") = 0
    

    Proposed patch adds the boolint converter which makes a conversion for you.

    closefd: boolint = True
    keepends: boolint = False
    

    @serhiy-storchaka serhiy-storchaka added topic-argument-clinic type-feature A feature request or enhancement labels Apr 23, 2015
    @larryhastings
    Copy link
    Contributor

    I think this is silly.

    Python has a well-understood concept of "truth":
    https://docs.python.org/3/library/stdtypes.html#truth-value-testing

    I assert that the reason people used the "i" format unit for what are really boolean values is because a) the "p" format unit wasn't added until very recently, and b) they were lazy and it was easy to copy-and-paste from other code.

    Rather than perpetuate these misguided hacks, when converting code to Argument Clinic we should convert them to properly support truth as defined in Python.

    @serhiy-storchaka
    Copy link
    Member Author

    I consider this as transitional style, for simpler converting to Argument Clinic without changing the behaviour. In future it can be replaced with bool.

    But the bool converter has a downside. It is harder to extend it. We can't add a support of say a tuple, or callable, or string, or ternary logic value without potential breaking some code. I propose first convert all semantic boolean ints to the boolint converter, and then, after fundamental analysis, replace boolint with bool if there is no any sense to extend concrete parameter.

    @taleinat
    Copy link
    Contributor

    If this was for internal use only, intended to ease the transition to Argument Clinic, then extensibility isn't an issue.

    An upside to this is that it would make it easy in the future to find all of the places in the stdlib using integers instead of bools.

    A downside is that "boolint" isn't a very obvious name IMO.

    @serhiy-storchaka
    Copy link
    Member Author

    I proposed boolint or intbool. Are there better variants?

    An alternative is add accept={int} parameter to the bool converter. But this will complicate the implementation and the use without the need.

    @taleinat
    Copy link
    Contributor

    If I was writing a function/method with a conceptually boolean parameter (True/False), I wouldn't want that to accept any Python object. For example, I would want passing a tuple or list to raise a TypeError. But according to the docs[1], that's what the 'p' converter does.

    If Python had a separate boolean type, this would be simple, but bool is a subclass of int, and it is easy to get a 0 or a 1 instead of True or False without noticing. So it seems reasonable when using the C API to accept an int when you want to get a boolean. I'd want a converter to convert it to a C bool, of course.

    @serhiy-storchaka
    Copy link
    Member Author

    Updated to the tip. Used new converter in recently converted _tkinter and _codecs modules. Now it is used 30 times.

    @serhiy-storchaka
    Copy link
    Member Author

    Updated patch uses bool(accept={int}) rather of boolint. It also updates more functions converted to Argument Clinic.

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 202fda5 by Serhiy Storchaka in branch 'master':
    bpo-24037: Add Argument Clinic converter bool(accept={int}). (#485)
    202fda5

    @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
    3.7 (EOL) end of life topic-argument-clinic type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants