Issue 25770: expose name, args, and kwargs from methodcaller

This patch adds 3 properties to methodcaller objects for inspecting the object at runtime:

1. 'name': the name of the method to call
2. 'args': the position arguments to pass to the method
3. 'keywords': the keyword arguments to pass to the method

args and keywords act like functools.partial (that is why I did not name it kwargs).

I noticed that recently the repr changed to expose this information which helps in the debugging use case; however, this allows us to use the methodcaller to maybe construct a new methodcaller with different args, or call a different method with the same args.

Hi,

I just tried your patch with the last revision and I have an error with the tests.

stephane@sg1 ~/s/h/cpython> ./python -m test test_operator
[1/1] test_operator
Fatal Python error: ./Modules/_operator.c:975 object at 0x7ff804c2e3d8 has negative ref count -1

Current thread 0x00007ff806ee8700 (most recent call first):
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/case.py", line 600 in run
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/case.py", line 648 in __call__
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/stephane/src/hg.python.org/cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/support/__init__.py", line 1679 in run
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/support/__init__.py", line 1780 in _run_suite
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/support/__init__.py", line 1814 in run_unittest
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/runtest.py", line 161 in test_runner
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/runtest.py", line 162 in runtest_inner
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/runtest.py", line 126 in runtest
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/main.py", line 295 in run_tests_sequential
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/main.py", line 356 in run_tests
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/main.py", line 392 in main
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/main.py", line 433 in main
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/libregrtest/main.py", line 455 in main_in_temp_cwd
  File "/home/stephane/src/hg.python.org/cpython/Lib/test/__main__.py", line 3 in <module>
  File "/home/stephane/src/hg.python.org/cpython/Lib/runpy.py", line 85 in _run_code
  File "/home/stephane/src/hg.python.org/cpython/Lib/runpy.py", line 170 in _run_module_as_main
fish: “./python -m test test_operator” terminated by signal SIGABRT (Abort)
stephane@sg1 ~/s/h/cpython> 


stephane@sg1 ~/s/h/cpython> hg tip
changeset:   99407:734247d5d0f9
tag:         tip
parent:      99404:ac0f7ed0e94d
parent:      99406:fee19d2d7713
user:        Zachary Ware <zachary.ware@gmail.com>
date:        Mon Nov 30 22:57:22 2015 -0600
summary:     Closes #25767: Merge with 3.5

Thanks for review, looking into the reference count issue.

Left a review comment which may help you chase that refleak

Also, as a new feature, surely it should be documented?

Thanks for pointing me at the refleak, uploading an update

Added a test case for the mutation of keywords.

This is a bit odd. You can mutate the keyword arguments, but you are not allowed to change the positional arguments (“args” is readonly). If you want this to be part of the supported API, it should be documented, but it seems like bad design IMO.

I only want this feature to keep the usage close to functools.partial. I was actually sort of surprised to see that mutation of the dict held in partial, but I would rather be consistent.

"keywords" is unusual name. The most used name for the dict of keyword arguments is "kwargs".

$ find Lib/ -name '*.py' -exec egrep -ho '\*\*[a-zA-Z_0-9]+' '{}' + | sort | uniq -c | sort -nr | head
    803 **kwargs
    442 **kw
    244 **kwds
...

If you want to have keyword arguments non-mutable, you can use types.MappingProxyType.

I agree that it is a strange name and I also think that it could be immutable or a copy of the internal dict; however, I think that consistency with existing APIs in the standard library is more important. 'keywords' is still very clear in context and is used in 'functools.partial'. This feature is very similar to 'partial' so I would like to follow the example set there.

I would not mind changing 'partial' to reflect this feedback; but, I imagine that will break people's code and be a harder sell. I would find it frustrating if 'partial' and 'methodcaller' spelled 'keywords' differently or had slightly different behavior when it comes to the keyword argument dictionary. Allowing the mutation is not that unintuitive if you look at the simple python translation I added to the docs.

functools.partial() is unique in it's usage of name "keywords".

partial's unique usage is why I feel like it would be so jarring for them do be named differently. I would be happiest having this feature at all so I will change the name to 'kwargs' if you would like. I just want to be sure that my reasons for choosing this name in the first place ere understood.

Is there a decision on the name? I can update the patch if needed.

ping: any decision on this?

I don't like the name "keywords". May be I'm not right. Needed opinions of other core developers.

Now that partial() has started down the path of using "keywords", it seems reasonable to stick with that name.  In pure python code, we mostly use **kwds or **kwargs but there are other variants and not much consistency, so spelling out keywords isn't unreasonable in that regard.  That said, if it feels weird to you, ask Guido what his thoughts are.

functools.partial is not the only precedence.

The "kwargs" attribute is used in following classes:

concurrent.futures.process._CallItem
concurrent.futures.process._WorkItem
concurrent.futures.thread._WorkItem
inspect.BoundArguments
sched.Event
threading.Timer
unittest.mock._patch
weakref.finalize._Info

The "kwds" attribute is used in following class: contextlib._GeneratorContextManager.

people have had some time to think about this, whats should the name be so we can merge this?

To me, the best rhythm has always been (*args, **kwds).

messages: + msg255684

messages: + msg255682

stage: patch review