Author steven.daprano
Recipients Alessandro Cucci, cheryl.sabella, steven.daprano
Date 2019-05-24.15:57:30
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1558713451.24.0.940626457256.issue36461@roundup.psfhosted.org>
In-reply-to
Content
(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.)
History
Date User Action Args
2019-05-24 15:57:31steven.dapranosetrecipients: + steven.daprano, cheryl.sabella, Alessandro Cucci
2019-05-24 15:57:31steven.dapranosetmessageid: <1558713451.24.0.940626457256.issue36461@roundup.psfhosted.org>
2019-05-24 15:57:31steven.dapranolinkissue36461 messages
2019-05-24 15:57:30steven.dapranocreate