msg255636 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-01 02:07 |
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.
|
msg255644 - (view) |
Author: Stéphane Wirtel (matrixise) * |
Date: 2015-12-01 09:09 |
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
|
msg255657 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-01 16:25 |
Thanks for review, looking into the reference count issue.
|
msg255680 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2015-12-01 22:08 |
Left a review comment which may help you chase that refleak
Also, as a new feature, surely it should be documented?
|
msg255682 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-01 23:05 |
Thanks for pointing me at the refleak, uploading an update
|
msg255684 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-01 23:28 |
Added a test case for the mutation of keywords.
|
msg255688 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2015-12-02 01:18 |
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.
|
msg255689 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-02 03:32 |
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.
|
msg255694 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-12-02 07:39 |
"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.
|
msg255695 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-02 07:52 |
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.
|
msg255696 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-12-02 08:00 |
functools.partial() is unique in it's usage of name "keywords".
|
msg255725 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-02 16:57 |
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.
|
msg256894 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2015-12-23 02:34 |
Is there a decision on the name? I can update the patch if needed.
|
msg260425 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2016-02-18 04:10 |
ping: any decision on this?
|
msg260764 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-02-24 08:03 |
I don't like the name "keywords". May be I'm not right. Needed opinions of other core developers.
|
msg260841 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2016-02-25 06:26 |
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.
|
msg260842 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-02-25 07:11 |
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.
|
msg264874 - (view) |
Author: Joe Jevnik (llllllllll) * |
Date: 2016-05-05 03:42 |
people have had some time to think about this, whats should the name be so we can merge this?
|
msg264927 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2016-05-05 23:15 |
To me, the best rhythm has always been (*args, **kwds).
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:24 | admin | set | github: 69956 |
2016-05-05 23:15:28 | gvanrossum | set | messages:
+ msg264927 |
2016-05-05 03:42:04 | llllllllll | set | messages:
+ msg264874 |
2016-02-25 07:11:41 | serhiy.storchaka | set | nosy:
+ gvanrossum messages:
+ msg260842
|
2016-02-25 06:26:51 | rhettinger | set | messages:
+ msg260841 |
2016-02-24 08:03:01 | serhiy.storchaka | set | messages:
+ msg260764 |
2016-02-18 04:10:28 | llllllllll | set | messages:
+ msg260425 |
2015-12-23 02:34:10 | llllllllll | set | messages:
+ msg256894 |
2015-12-02 16:57:24 | llllllllll | set | messages:
+ msg255725 |
2015-12-02 08:00:32 | serhiy.storchaka | set | nosy:
+ rhettinger, ncoghlan messages:
+ msg255696
|
2015-12-02 07:52:37 | llllllllll | set | messages:
+ msg255695 |
2015-12-02 07:39:30 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg255694
|
2015-12-02 03:32:04 | llllllllll | set | messages:
+ msg255689 |
2015-12-02 01:18:36 | martin.panter | set | messages:
+ msg255688 |
2015-12-01 23:28:52 | llllllllll | set | files:
+ methodcaller-attrs-2.patch
messages:
+ msg255684 |
2015-12-01 23:05:18 | llllllllll | set | files:
+ methodcaller-attrs-1.patch
messages:
+ msg255682 |
2015-12-01 22:08:38 | martin.panter | set | nosy:
+ martin.panter messages:
+ msg255680
|
2015-12-01 16:25:48 | llllllllll | set | messages:
+ msg255657 |
2015-12-01 09:09:42 | matrixise | set | nosy:
+ matrixise messages:
+ msg255644
|
2015-12-01 02:17:09 | abarry | set | nosy:
+ abarry
stage: patch review |
2015-12-01 02:07:11 | llllllllll | create | |