On 03/03/2015 11:00, Paulo Ricardo Paz Vital wrote:
On Tue, 2015-03-03 at 10:21 -0300, CrÃstian Viana 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
On 03-03-2015 05:16, Paulo Ricardo Paz Vital wrote: 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel