Author lukasz.langa
Recipients fdrake, georg.brandl, lukasz.langa, michael.foord, pitrou
Date 2010-08-10.12:28:30
SpamBayes Score 1.11022e-16
Marked as misclassified No
Message-id <D344ADAA-F709-4A23-BCB8-28755B551B78@langa.pl>
In-reply-to <1281438944.82.0.450024833619.issue9421@psf.upfronthosting.co.za>
Content
I am away from the computer at the moment but doing my best to reply.

> - Using _UNSET & similar in the docs is not good; there used to be a
>  way to note a parameter as optional.  Not sure whether there is any
>  more.

I don't know where to look for something like it. How's 'range' documented?

> - Docs for methods which take vars/default should include a note
>  indicating where to find the explanation, since reference docs are
>  often read in a non-linear manner.

Agreed, will edit the patch.

> - This is a tortured way to only do something in a particular condition:
> 
>    value = str(value) if value is not None else None
> 
>  These two lines are actually more readable, since it's clear that
>  the branch is a no-op:
> 
>    if value is not None:
>        value = str(value)

Right. Thanks, will include this.

> - _unify_boolean is a strange name; doesn't parallel _unify_values at
>  all.

_convert_to_boolean?

> - Having _COMPATIBLE and _UNSET defined in the classes looks more
>  painful than not; no need to change for this patch.  (Should be
>  changed in some later cleanup.)  Moving the whole _unify_boolean /
>  _boolean_states mess to a separate top-level function that can be
>  used for the conversion is attractive.
> 

Will do that in a separate patch. Are you aware that moving _boolean_states out of the class is slightly backwards incompatible? I say "slightly" because it's private after all.
History
Date User Action Args
2010-08-10 12:28:32lukasz.langasetrecipients: + lukasz.langa, fdrake, georg.brandl, pitrou, michael.foord
2010-08-10 12:28:30lukasz.langalinkissue9421 messages
2010-08-10 12:28:30lukasz.langacreate