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

Allow decimal.localcontext to accept keyword arguments to set context attributes #91291

Closed
stevendaprano opened this issue Mar 27, 2022 · 8 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir

Comments

@stevendaprano
Copy link
Member

BPO 47135
Nosy @ncoghlan, @stevendaprano, @corona10, @dignissimus
PRs
  • gh-91291: Accept attributes as keyword arguments in decimal.localcontext #32242
  • Files
  • sam_ezeh.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 = None
    created_at = <Date 2022-03-27.10:16:04.212>
    labels = ['library', '3.11']
    title = 'Allow decimal.localcontext to accept keyword arguments to set context attributes'
    updated_at = <Date 2022-04-01.20:49:19.136>
    user = 'https://github.com/stevendaprano'

    bugs.python.org fields:

    activity = <Date 2022-04-01.20:49:19.136>
    actor = 'sam_ezeh'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2022-03-27.10:16:04.212>
    creator = 'steven.daprano'
    dependencies = []
    files = ['50713']
    hgrepos = []
    issue_num = 47135
    keywords = ['patch']
    message_count = 6.0
    messages = ['416114', '416364', '416507', '416511', '416513', '416514']
    nosy_count = 4.0
    nosy_names = ['ncoghlan', 'steven.daprano', 'corona10', 'sam_ezeh']
    pr_nums = ['32242']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue47135'
    versions = ['Python 3.11']

    @stevendaprano
    Copy link
    Member Author

    Whenever I use decimal and need to change the context temporarily, without fail I try to write

    with decimal.localcontext(prec=10):
        ...

    or similar. Then I get surprised that it fails, and re-write it as

    with decimal.localcontext() as ctx:
        ctx.prec = 10
        ...

    Let's make the first version work. localcontext should accept keyword arguments corresponding to the same arguments accepted by Context, and set them on the context given.

    A proof-of-concept wrapper function:

    def localcontext(ctx=None, **kwargs):
        if ctx is None:
            ctx = decimal.getcontext().copy()
        for key, value in kwargs.items():
            setattr(ctx, key, value)
        return decimal.localcontext(ctx)

    I think this would be a valuable, and useful, improvement to the decimal API.

    @stevendaprano stevendaprano added 3.11 only security fixes stdlib Python modules in the Lib dir labels Mar 27, 2022
    @ncoghlan
    Copy link
    Contributor

    Seems reasonable to me, but I think a full implementation would want to throw an error for keyword args that don't already exist as context attributes (otherwise typos would fail silently)

    @dignissimus
    Copy link
    Mannequin

    dignissimus mannequin commented Apr 1, 2022

    I'm looking into adding this

    Seems reasonable to me, but I think a full implementation would want to throw an error for keyword args that don't already exist as context attributes (otherwise typos would fail silently)

    For _pydecimal, I think this would automatically happen automatically as Context.__setattr__ raises AttributeError when it's passed a name that isn't a context attribute.

    For _decimal this can be done with keyword arguments and PyArg_ParseTupleAndKeywords.

    @dignissimus
    Copy link
    Mannequin

    dignissimus mannequin commented Apr 1, 2022

    I've uploaded a patch and it seems to work, which I'm very proud of.

    I'll create some tests, amend documentation and create a news entry. After that, I'll create a pull request on GitHub.

    @stevendaprano
    Copy link
    Member Author

    I'm not sure what the implementation uses to enforce this, but decimal
    contexts already seem to reject arbitrary attributes. So a naive
    implementation that just setattr()'s the keyword arguments will
    automatically fail:

    >>> from decimal import getcontext
    >>> ctx = getcontext()
    >>> setattr(ctx, 'precision', 10)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    AttributeError: 'decimal.Context' object has no attribute 'precision'

    But you are absolutely correct that however we enforce it, we should
    avoid allowing typos to silently fail.

    @dignissimus
    Copy link
    Mannequin

    dignissimus mannequin commented Apr 1, 2022

    This is what functionality looks like when supplying incorrect attribute names with the patch.

    Python 3.11.0a6+ (heads/bpo-47135-dirty:d4bb38f82b, Apr  1 2022, 20:01:56) [GCC 11.1.0] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import _pydecimal
    >>> ctx = _pydecimal.getcontext()
    >>> ctx.precision = 10
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/run/media/sam/OS/Git/cpython/Lib/_pydecimal.py", line 3974, in __setattr__
        raise AttributeError(
        ^^^^^^^^^^^^^^^^^^^^^
    AttributeError: 'decimal.Context' object has no attribute 'precision'
    >>> with _pydecimal.localcontext(precision=10) as ctx:
    ...     pass
    ... 
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/run/media/sam/OS/Git/cpython/Lib/_pydecimal.py", line 506, in localcontext
        setattr(ctx, key, value)
        ^^^^^^^^^^^^^^^^^^^^^^^^
      File "/run/media/sam/OS/Git/cpython/Lib/_pydecimal.py", line 3974, in __setattr__
        raise AttributeError(
        ^^^^^^^^^^^^^^^^^^^^^
    AttributeError: 'decimal.Context' object has no attribute 'precision'
    >>> import decimal
    >>> ctx = decimal.getcontext()
    >>> ctx.precision = 10
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    AttributeError: 'decimal.Context' object has no attribute 'precision'
    >>> with decimal.localcontext(precision=10) as ctx:
    ...     pass
    ... 
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: 'precision' is an invalid keyword argument for this function
    >>>

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    JelleZijlstra added a commit that referenced this issue Apr 22, 2022
    …ext (#32242)
    
    Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
    @JelleZijlstra
    Copy link
    Member

    Implemented in #32242. Thanks for the PR!

    @jkloth
    Copy link
    Contributor

    jkloth commented Apr 22, 2022

    It appears this change is also causing issues on the entire buildbot fleet

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants