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

BoundArguments.arguments used in the recommended way to call a callable silently succeeds for nonexistent arguments #85911

Open
Julian mannequin opened this issue Sep 8, 2020 · 7 comments
Labels
3.10 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@Julian
Copy link
Mannequin

Julian mannequin commented Sep 8, 2020

BPO 41745
Nosy @terryjreedy, @Julian, @1st1

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 2020-09-08.15:40:46.385>
labels = ['type-bug', '3.10', 'docs']
title = 'BoundArguments.arguments used in the recommended way to call a callable silently succeeds for nonexistent arguments'
updated_at = <Date 2020-09-13.05:07:36.438>
user = 'https://github.com/Julian'

bugs.python.org fields:

activity = <Date 2020-09-13.05:07:36.438>
actor = 'terry.reedy'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation']
creation = <Date 2020-09-08.15:40:46.385>
creator = 'Julian'
dependencies = []
files = []
hgrepos = []
issue_num = 41745
keywords = []
message_count = 7.0
messages = ['376578', '376590', '376755', '376756', '376778', '376809', '376823']
nosy_count = 4.0
nosy_names = ['terry.reedy', 'docs@python', 'Julian', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue41745'
versions = ['Python 3.10']

@Julian
Copy link
Mannequin Author

Julian mannequin commented Sep 8, 2020

The following code succeeds "silently", which seems undesirable:

    from inspect import signature
    def two():
        return 2
    bound = signature(two).bind()
    bound.arguments["does_not_exist"] = 12
    two(*bound.args, **bound.kwargs)

where the mechanism for finally calling two is the recommended way shown in the docs for inspect.BoundArguments: https://docs.python.org/3/library/inspect.html#inspect.BoundArguments.apply_defaults

What's happened there is that:

    print(b.args, b.kwargs)

"silently" ignored the non-existent argument.

Somewhere along the line here it seems like something should complain. I don't see previous discussion of this from quickly searching on the bug tracker, but obviously if I've missed something let me know.

I'm also not really sure what the desirable solution is. To me, it's possibly that BoundArguments should have a fully-managed way to invoke a callable rather than asking a user to unpack *args and *kwargs, and that that mechanism, say arguments.be_passed_to(callable) should do the error checking). Having .arguments full-on reject unknown parameters seems like another possibility but there may be reasons that's not a good idea (e.g. if you were to for some reason use a bound arguments object to call some other function that did take that additional argument).

@Julian Julian mannequin added 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir 3.10 only security fixes type-bug An unexpected behavior, bug, or error and removed 3.8 only security fixes 3.9 only security fixes labels Sep 8, 2020
@Julian
Copy link
Mannequin Author

Julian mannequin commented Sep 8, 2020

As a secondary behavior here, which is actually the one that matters more for my use case, the following seems surprising as well:

import inspect
s = inspect.signature(lambda **kwargs: kwargs).bind()
s.arguments["foo"] = 12

will similarly silently drop "foo" when attempting to call the function (i.e. it will not appear in bound.kwargs).

This behavior I suspect is the intention of the docs saying "Contains only explicitly bound arguments." but still seems easily misused when .arguments is being used as a frontend for adding additional arguments.

To me just brainstorming again it seems it'd be useful to provide a harder-to-misuse frontend, e.g. BoundArguments.with_arguments(*args, **kwargs) that returns a bound arguments with some additional to-be-bound arguments which may end up in kwargs.

@terryjreedy
Copy link
Member

Bound is created with 5 public attributes:
>>> dir(bound)
[..., 'apply_defaults', 'args', 'arguments', 'kwargs', 'signature']
>>> bound.args
()
>>> bound.arguments
{}
>>> bound.kwargs
{}
msg376578: I don't understand 'non-existent' arguments,  Nor 'what happened...print... ignored' as there is no previous print.

msg376590: Given " Changes in arguments will reflect in args and kwargs.", I agree that changes to 'arguments' *apparently* not being reflected in 'args' and 'kwargs' is initally a bit puzzling
.
>>> bound.kwargs == bound.arguments
True
>>> bound.arguments['something'] = 'guess'
>>> bound.kwargs
{}
>>> bound.arguments
{'something': 'guess'}

However, your 'two' function takes no arguments, so valid values of args and kwargs must be empty for them to be used in a call. In all cases, args() and kwargs() must look at the signature to see which key-value pairs they should extract from arguments.

>> def f(a): pass

>>> signature(f).bind()  # Must pass value arguments
Traceback (most recent call last):
...
TypeError: missing a required argument: 'a'
>>> b = signature(f).bind(3)
>>> b.arguments
{'a': 3}
>>> b.args
(3,)  # Because 'a' is positional.
>>> b.kwargs
{}  # Because 'a' is not keyword only.
>>> b.arguments['a']=5
>>> b.args
(5,)  # Legitimate change reflected here.

Perhaps the doc could be improved, but I have no particular suggestion.

@terryjreedy terryjreedy added docs Documentation in the Doc dir and removed stdlib Python modules in the Lib dir labels Sep 12, 2020
@Julian
Copy link
Mannequin Author

Julian mannequin commented Sep 12, 2020

Not sure I agree with it being just a doc issue -- happy to clarify if something was unclear, not sure from your message if it was or if you disagree, but e.g.:

However, your 'two' function takes no arguments, so valid values of args and kwargs must be empty for them to be used in a call. In all cases, args() and kwargs() must look at the signature to see which key-value pairs they should extract from arguments.

Right, and that's "dangerous" behavior to me -- BoundArguments.arguments will happily let you add arguments to it, but does no error checking at that point, so does not raise an exception if you typo an argument that isn't in the signature of the callable, and then when you try to call the callable (via the only way possible, namely using .args and .kwargs), that argument that was never added is just dropped.

Does that make sense? Or were you disagreeing with that being undesirable behavior?

@terryjreedy
Copy link
Member

I use signature for IDLE call tips, but have never used .bind and subsequent calls. After understanding the behavior, my question as initial reviewer is whether it is an *implementation* bug. Perhaps, but not obviously so.

Whether or not it is a design wart serious enough to warrant an design change is a different question, which I am not going to judge. Maybe there is a reason for the current design we have not thought of. Or maybe it is an accident too painful to change.

I am convinced that the doc could be better.

@Julian
Copy link
Mannequin Author

Julian mannequin commented Sep 12, 2020

Totally fair! Sorry, was just making sure the label change wasn't intended to say it *wasn't* enough to warrant a design change :) (which I agree should be discussed with folks who do use that functionality, of which I only recently had to).

@terryjreedy
Copy link
Member

If you want to pursue, you might try python-list to find other users. I am not sure if this change really qualifies for python-ideas.

@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.10 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant