Re: [Python-Dev] [Python-checkins] cpython (3.3): Issue #16641: Fix default values of sched.scheduler.enter arguments were
On Sat, Dec 29, 2012 at 11:17 AM, serhiy.storchaka < python-checkins@python.org> wrote:
http://hg.python.org/cpython/rev/1c9c0f92df65 changeset: 81134:1c9c0f92df65 branch: 3.3 parent: 81132:5db0833f135b user: Serhiy Storchaka <storchaka@gmail.com> date: Sat Dec 29 21:13:45 2012 +0200 summary: Issue #16641: Fix default values of sched.scheduler.enter arguments were modifiable.
files: Doc/library/sched.rst | 23 ++++++++++++++--------- Lib/sched.py | 8 ++++++-- Misc/NEWS | 3 +++ 3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/Doc/library/sched.rst b/Doc/library/sched.rst --- a/Doc/library/sched.rst +++ b/Doc/library/sched.rst @@ -36,19 +36,22 @@
>>> import sched, time >>> s = sched.scheduler(time.time, time.sleep) - >>> def print_time(): print("From print_time", time.time()) + >>> def print_time(a='default'): + ... print("From print_time", time.time(), a) ... >>> def print_some_times(): ... print(time.time()) - ... s.enter(5, 1, print_time, ()) - ... s.enter(10, 1, print_time, ()) + ... s.enter(10, 1, print_time) + ... s.enter(5, 2, print_time, argument=('positional',)) + ... s.enter(5, 1, print_time, kwargs={'a': 'keyword'}) ... s.run() ... print(time.time()) ... >>> print_some_times() 930343690.257 - From print_time 930343695.274 - From print_time 930343700.273 + From print_time 930343695.274 positional + From print_time 930343695.275 keyword + From print_time 930343700.273 default 930343700.276
.. _scheduler-objects: @@ -59,7 +62,7 @@ :class:`scheduler` instances have the following methods and attributes:
-.. method:: scheduler.enterabs(time, priority, action, argument=[], kwargs={}) +.. method:: scheduler.enterabs(time, priority, action, argument=(), kwargs={})
Schedule a new event. The *time* argument should be a numeric type compatible with the return value of the *timefunc* function passed to the constructor. @@ -67,8 +70,10 @@ *priority*.
Executing the event means executing ``action(*argument, **kwargs)``. - *argument* must be a sequence holding the parameters for *action*. - *kwargs* must be a dictionary holding the keyword parameters for *action*. + Optional *argument* argument must be a sequence holding the parameters + for *action* if any used. + Optional *kwargs* argument must be a dictionary holding the keyword + parameters for *action* if any used.
I don't see how this change improves the documentation. To keep the grammar correct and just state that the arguments are optional, I would simply replace "must be" by "is". For example: *argument* is a sequence holding the parameters for *action*. This is short, and since the function signature clearly shows that argument has a default value, I think it conveys the meaning it should. Eli
On 12/29/2012 08:32 PM, Eli Bendersky wrote:
On Sat, Dec 29, 2012 at 11:17 AM, serhiy.storchaka <python-checkins@python.org <mailto:python-checkins@python.org>> wrote:
http://hg.python.org/cpython/rev/1c9c0f92df65 changeset: 81134:1c9c0f92df65 branch: 3.3 parent: 81132:5db0833f135b user: Serhiy Storchaka <storchaka@gmail.com <mailto:storchaka@gmail.com>> date: Sat Dec 29 21:13:45 2012 +0200 summary: Issue #16641: Fix default values of sched.scheduler.enter arguments were modifiable.
files: Doc/library/sched.rst | 23 ++++++++++++++--------- Lib/sched.py | 8 ++++++-- Misc/NEWS | 3 +++ 3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/Doc/library/sched.rst b/Doc/library/sched.rst --- a/Doc/library/sched.rst +++ b/Doc/library/sched.rst @@ -36,19 +36,22 @@
>>> import sched, time >>> s = sched.scheduler(time.time, time.sleep) - >>> def print_time(): print("From print_time", time.time()) + >>> def print_time(a='default'): + ... print("From print_time", time.time(), a) ... >>> def print_some_times(): ... print(time.time()) - ... s.enter(5, 1, print_time, ()) - ... s.enter(10, 1, print_time, ()) + ... s.enter(10, 1, print_time) + ... s.enter(5, 2, print_time, argument=('positional',)) + ... s.enter(5, 1, print_time, kwargs={'a': 'keyword'}) ... s.run() ... print(time.time()) ... >>> print_some_times() 930343690.257 - From print_time 930343695.274 - From print_time 930343700.273 + From print_time 930343695.274 positional + From print_time 930343695.275 keyword + From print_time 930343700.273 default 930343700.276
.. _scheduler-objects: @@ -59,7 +62,7 @@ :class:`scheduler` instances have the following methods and attributes:
-.. method:: scheduler.enterabs(time, priority, action, argument=[], kwargs={}) +.. method:: scheduler.enterabs(time, priority, action, argument=(), kwargs={})
Schedule a new event. The *time* argument should be a numeric type compatible with the return value of the *timefunc* function passed to the constructor. @@ -67,8 +70,10 @@ *priority*.
Executing the event means executing ``action(*argument, **kwargs)``. - *argument* must be a sequence holding the parameters for *action*. - *kwargs* must be a dictionary holding the keyword parameters for *action*. + Optional *argument* argument must be a sequence holding the parameters + for *action* if any used. + Optional *kwargs* argument must be a dictionary holding the keyword + parameters for *action* if any used.
I don't see how this change improves the documentation. To keep the grammar correct and just state that the arguments are optional, I would simply replace "must be" by "is". For example:
*argument* is a sequence holding the parameters for *action*.
This is short, and since the function signature clearly shows that argument has a default value, I think it conveys the meaning it should.
Hi Eli, I'm sure we non-native speakers are fine with any improvements you can make during commit review. Georg
Executing the event means executing ``action(*argument,
**kwargs)``.
- *argument* must be a sequence holding the parameters for
*action*.
- *kwargs* must be a dictionary holding the keyword parameters for
*action*.
+ Optional *argument* argument must be a sequence holding the
parameters
+ for *action* if any used. + Optional *kwargs* argument must be a dictionary holding the
keyword
+ parameters for *action* if any used.
I don't see how this change improves the documentation. To keep the
grammar
correct and just state that the arguments are optional, I would simply replace "must be" by "is". For example:
*argument* is a sequence holding the parameters for *action*.
This is short, and since the function signature clearly shows that argument has a default value, I think it conveys the meaning it should.
Hi Eli,
I'm sure we non-native speakers are fine with any improvements you can make during commit review.
Georg
Georg, I also wrote a private email to Serhiy proposing to help, but since you brought this up here: I think that my comment was constructive. What should have I done differently? Go ahead and modify the phrasing in a separate commit? I see a couple of problems with that: 1. It can be somewhat disrespectful to a new committer, and I wanted to reach a consensus first. 2. Serhiy diligently committed this into 3 or 4 different Python branches. With all due respect, going through the merge/push dance is far above the effort I'm willing to invest in this. Eli P.S. I would argue that you are more native-speaker than myself w.r.t. English :-)
On 12/29/2012 10:44 PM, Eli Bendersky wrote:
> Executing the event means executing ``action(*argument, **kwargs)``. > - *argument* must be a sequence holding the parameters for *action*. > - *kwargs* must be a dictionary holding the keyword parameters for *action*. > + Optional *argument* argument must be a sequence holding the parameters > + for *action* if any used. > + Optional *kwargs* argument must be a dictionary holding the keyword > + parameters for *action* if any used. > > > I don't see how this change improves the documentation. To keep the grammar > correct and just state that the arguments are optional, I would simply replace > "must be" by "is". For example: > > *argument* is a sequence holding the parameters for *action*. > > This is short, and since the function signature clearly shows that argument has > a default value, I think it conveys the meaning it should.
Hi Eli,
I'm sure we non-native speakers are fine with any improvements you can make during commit review.
Georg
Georg,
I also wrote a private email to Serhiy proposing to help, but since you brought this up here: I think that my comment was constructive. What should have I done differently?
Nothing at all. In case you read my comment as sarcasm, please don't. It probably was a little unreflected.
Go ahead and modify the phrasing in a separate commit? I see a couple of problems with that:
1. It can be somewhat disrespectful to a new committer, and I wanted to reach a consensus first.
I would not feel unrespected by someone correcting nits in my English grammar. (Maybe it would be different for Python grammar :)
2. Serhiy diligently committed this into 3 or 4 different Python branches. With all due respect, going through the merge/push dance is far above the effort I'm willing to invest in this.
I understand. I probably would have not done it instantly either, but put it on my todo list and committed when I had some other change as well.
Eli
P.S. I would argue that you are more native-speaker than myself w.r.t. English :-)
Then I was mistaken there. (Are there multiple levels of nativeness? :) Or is it because English is half-Germanic?) cheers, Georg
participants (2)
-
Eli Bendersky
-
Georg Brandl