Steven D'Aprano <steve+pyt...@pearwood.info> 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 <rep...@bugs.python.org>
<https://bugs.python.org/issue36461>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to