4:17 p.m.
On 03/03/2015 11:00, Paulo Ricardo Paz Vital wrote:
On Tue, 2015-03-03 at 10:21 -0300, CrÃstian Viana wrote:
> On 03-03-2015 05:16, Paulo Ricardo Paz Vital wrote:
>> s/src_unit/src_unit=None
>> As explained below, both second and third arguments can be None and then
>> automatically converted to use DEFAULT_UNIT.
>>
>> I tried "convert_data_size(4)" and got the error:
>> TypeError: convert_data_size() takes at least 2 arguments (1 given)
> I didn't mean the first parameter to be optional. What would you expect
> when calling "convert_data_size(4)"? Assuming both units as bytes, the
> function would always return the same value which was passed to it. Both
> parameters *can* be None but that doesn't mean we should create a
> default situation where it happens, because that wouldn't be useful at all.
>
OK, so I think you should do some modifications on code to be coherent
with what you intend to do.
First, you should re-write the documentation part where you explain the
parameters, by removing the line "if 'src_unit' is empty, the unit
'B' (byte) will be used.", because it will never happen.
Second, it's not necessary the following block, again, because it will
never happen. Removing this block, you will guarantee that 'src_unit'
and 'dst_unit' will be always different and you will not create a
default situatio.
+ if not src_unit:
+ src_unit = DEFAULT_UNIT
I don't think we should use None as a default parameter and then covert
it to a real parameter.
It should use the real parameter in function.
def convert_data_size(value, src_unit, dst_unit='B'):
And make the first and second parameters required. So we always need to
pass the value and its unit to do the conversion.
And the default destination unit is 'B'.
>>> + SI_PREFIXES = ['k', 'M', 'G', 'T',
'P', 'E', 'Z', 'Y']
>> I guess you can add a new line here, as you did between
>> SUFFIXES_WITH_MULT and DEFAULT_SUFFIX.
>>
>>> + # The IEC prefixes are the equivalent SI prefixes + 'i'
>>> + # but, exceptionally, 'k' becomes 'Ki' instead of
'ki'.
>>> + IEC_PREFIXES = map(lambda p: 'Ki' if p == 'k' else p +
'i', SI_PREFIXES)
>> New line here.
>>
>>> + PREFIXES_BY_BASE = {1000: SI_PREFIXES,
>>> + 1024: IEC_PREFIXES}
>>> +
>>> + SUFFIXES_WITH_MULT = {'b': 1,
>>> + 'B': 8}
>>> + DEFAULT_SUFFIX = 'B'
>>> +
>>> + DEFAULT_UNIT = 'B'
>>> +
>>> +
> Well, I tried to group the variable declarations by subject:
> prefix-related variables, <blank space>, suffix-related variables,
> <blank space>, unit-related variable.
Got it!
However, I'm "old school" and learned that if you add a comment, you, as
a best practice, should add a blank line between the previous line and
the comment line :-P
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel