Message113529
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. |
|
Date |
User |
Action |
Args |
2010-08-10 12:28:32 | lukasz.langa | set | recipients:
+ lukasz.langa, fdrake, georg.brandl, pitrou, michael.foord |
2010-08-10 12:28:30 | lukasz.langa | link | issue9421 messages |
2010-08-10 12:28:30 | lukasz.langa | create | |
|