[issue36461] timeit: Additional changes for autorange

[Steven D'Aprano]

Steven D'Aprano  added the comment:

> ``if number is None or number < 0``

Sorry, that should be number == 0

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Steven D'Aprano]

Steven D'Aprano  added the comment:

Sorry for the late reply.

> Just a question: why we need to check ``if number == 0:``? In the 
> proposal you asked for None too. What changed? Even if the function is 
> called with False, will it hurts to keep the default value?

Fair question. On rethinking, I'm okay with an explicit check for None 
or zero, ``if number is None or number < 0`` but I don't like the idea 
of accepting *any* falsey value.

Calling the function with False is fine, since False == 0 but I don't 
think it is fine to call the function with (say) [] or {} or "", which 
are all falsey values.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Michele Angrisano]

Michele Angrisano  added the comment:

I agree with *target_time*. I'm working on it and soon I'm going to update the 
pr.

--
nosy: +mangrisano

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Alessandro Cucci]

Alessandro Cucci  added the comment:

Thanks Steven, I like the new name "target_time".

Just a question: why we need to check ``if number == 0:``? In the proposal you 
asked for None too. What changed? Even if the function is called with False, 
will it hurts to keep the default value?

I'll try to implement your ideas during this weekend.

--
nosy: +acucci -Alessandro Cucci

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Steven D'Aprano]

Steven D'Aprano  added the comment:

Michele, Alessandro, thank you both for your work! And thank you Cheryl for 
managing this ticket.

I like mangrisano's design where the target time is passed as an argument to 
the ``autorange`` method, although I prefer the name "target_time". (But I'm 
open to alternatives.)

So I think a good approach might be a hybrid between mangrisano's design, and 
the design by Alessandro:

1. the constructor takes the target time and records it as an attribute;

def __init__(self, ..., target_time=default_target_time):
self.target_time = target_time


2. when the ``timeit`` method is used, if number=0 the target time is taken 
from self.target_time and passed to ``autorange``;

- if the ``autorange`` method is called directly, the caller can override the 
target time by passing it as argument; otherwise the default value is looked up 
from the instance attribute.

So something like

def autorange(self, callback=None, target_time=None):
if target_time is None:
target_time = self.target_time
...


Please suggest alternatives or point out anything I may have missed, and thank 
you all again!

(I'm now going to be away from the keyboard for at least the next 18 hours so 
if I don't reply quickly, that's why.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Steven D'Aprano]

Steven D'Aprano  added the comment:

Looking at PR 12953:

The only API for setting the target time is by passing it to the autorange 
method directly. So I think that there's no way for the caller of 
``Timer.timeit()`` or ``Timer.repeat()`` to specify a custom target time, is 
that right?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Steven D'Aprano]

Steven D'Aprano  added the comment:

(Sorry, I can't comment on Github.)

Looking at PR 12954:

I'm not sure about the API for making the time used by autorange configurable. 
The time taken is only used when autoranging, but the API takes it as an 
argument to the constructor even if it won't be used. I haven't thought deeply 
about this for a few years, so now I'm wondering if this is the best API. Let 
me think some more.

However, the parameter name "max_time_taken" is certainly wrong: it can be 
wrongly read as meaning the maximum time the sample code will run, which is not 
the case. 

Also, it is not a maximum time, it is a *minimum* time: the autoranger 
increases the number of loops until it takes *at least* ``max_time_taken`` 
seconds, not at most.

So I think a better name might be something like ``target_time``. It suggests a 
time we are targeting, not one we promise to hit precisely, it doesn't mislead 
into thinking it will be the total time. And it is a bit shorter without being 
cryptically short.

Anyone have any better suggestions for the parameter name?

Likewise I suggest ``default_target_time`` for the global (line 62).


With the longer parameter list, there's a couple of places that the line length 
seems to exceed 79 columns, e.g. lines 244, 272.

Line 176: I think that should be ``if number == 0:`` since we don't want to 
accept any arbitrary falsey value, just zero.


NEWS line 4: take out the reference to None.

(I haven't reviewed the tests at this stage.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Steven D'Aprano]

Steven D'Aprano  added the comment:

> @steven.daprano, would you be able to review the pull requests based 
> on your original concept for this change?  Thank you!

With difficulty... for technology/financial reasons, I don't have a 
browser that works properly with github. But I'll do what I can.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Cheryl Sabella]

Cheryl Sabella  added the comment:

@steven.daprano, would you be able to review the pull requests based on your 
original concept for this change?  Thank you!

--
assignee: Mariatta -> 
nosy:  -Mariatta

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Alessandro Cucci]

Change by Alessandro Cucci :


--
pull_requests: +12880

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Michele Angrisano]

Change by Michele Angrisano :


--
keywords: +patch
pull_requests: +12879
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Alessandro Cucci]

Alessandro Cucci  added the comment:

Hello @Mariatta,
if this is simple I would like to work on that, can I?
Thanks!

--
nosy: +Alessandro Cucci

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Cheryl Sabella]

Change by Cheryl Sabella :


--
nosy: +steven.daprano

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Cheryl Sabella]

Cheryl Sabella  added the comment:

Assigning to @Mariatta for the sprints.

--
assignee:  -> Mariatta
nosy: +Mariatta

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36461] timeit: Additional changes for autorange

[Cheryl Sabella]

New submission from Cheryl Sabella :

#6422 implemented the autorange function for timeit, but in msg272704, Steven 
D'Aprano outlined follow-up change requests to that patch.

- make the 0.2s time configurable;
- have `timeit` and `repeat` methods (and functions) fall back 
  on `autorange` if the number is set to 0 or None.

Opening this ticket for those change requests.

--
components: Library (Lib)
keywords: easy
messages: 339025
nosy: cheryl.sabella
priority: normal
severity: normal
stage: needs patch
status: open
title: timeit: Additional changes for autorange
type: enhancement
versions: Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com