Fix units in fs.filesize

[althonos]

According to Wikipedia, the binary filesize units should be KiB, MiB, ..., and the decimal units should be kB, MB, ...

Previously, traditional would use the SI prefixes, and decimal would display bits instead of bytes.

Also, I'm not sure about this, but once again functions should probably be raising TypeError when given an argument that cannot be coerced to a numerical value.

[coveralls]

Coverage remained the same at 99.949% when pulling f02c447 on althonos:patch-1 into 7a274da on PyFilesystem:master.

[willmcgugan]

I was using https://en.wikipedia.org/wiki/File_size as a guide, which appears to contradict with that.

It's all very confusing, tbh.

Think you're right re ValueError -> TypeError.

[althonos]

I tried to read https://en.wikipedia.org/wiki/Binary_prefix and it did not make more sense:

If you check the IEC, it enforces the use of Ki, Mi, etc. for binary sizes, but the JDEC goes the other way around, that's why nobody is trying to make things clearer.

But it seems like Kibi means Kilobinary, so I guess it should be used with binary.

[willmcgugan]

The original implementation matches this django filter.

I'm thinking that perhaps filesize.py doesn't belong here. I use it in moya serve when rendering directory listings, but it isn't referenced anywhere in fs.

Are you using it anywhere? Tempted to just remove if from the lib to sidestep this thorny issue.

[althonos]

I wouldn't remove it, since this is the kind of code you keep on rewriting and rewriting again everytime you need it, it's good that it's available in the library somewhere IMO.

But still, even the Django doc states that they are not using the right prefixes, so...

[willmcgugan]

Thing is I'd rather not be descriptive regarding these units. There will always be developers who will want to do it the 'wrong' way to match some pre-existing convention perhaps.

Maybe the best approach would be to provide a way of using any unit system you like, but default to the more technically correct way.

BTW do you still favour the units in the PR or are you confused as I am? Django likes power of two for file sizes, OSX likes powers of 10.

[althonos]

Then let's name the functions after what they do, and not what standard they try to conform to: we rename filesize.traditional to filesize.binary. Then, we add a kwarg named si to fs.binary that defaults to True, to choose if we want the prefix or not.

>>> fs.filesize.binary(4096)
4.0 KiB
>>> fs.filesize.binary(4096, si=False):
4.0 KB
>>> fs.filesize.decimal(4096)
4.1 kB

Here: KiB is the SI / ISO standard for binary, kB is the SI for decimal, and KB is the JDEC standard for binary.

[willmcgugan]

That sounds like a plan. How would you feel about leaving in traditional as it is at the moment, so that it wouldn't break moya serve?

[althonos]

• fs.filesize.traditional(size) will return fs.filesize.binary(size, si=False).
• filesize functions will raise TypeError.

[coveralls]

Coverage increased (+4.0e-05%) to 99.949% when pulling bf4f1ac on althonos:patch-1 into 7a274da on PyFilesystem:master.

[willmcgugan]

How about this as an alternative to line continuation...

prefixes = (
    ('KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB')
    if si else
    ('KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB')
)

[coveralls]

Coverage increased (+4.0e-05%) to 99.949% when pulling 75cd5a0 on althonos:patch-1 into 7a274da on PyFilesystem:master.

[willmcgugan]

Looks great. Just needs "binary" in all. Happy to merge if you are?

[althonos]

I realize, if we are to keep traditionnal, then maybe we don't need the si kwarg, and just advertise everything in the documentation.

• traditionnal --> KB, MB, GB, .... /1024
• binary --> KiB, MiB, GiB, .... /1024
• decimal --> kB, MB, GB, .... /1000

(this looks like an endless debate 😄)

[coveralls]

Coverage increased (+4.0e-05%) to 99.949% when pulling 05c960d on althonos:patch-1 into 7a274da on PyFilesystem:master.

[willmcgugan]

The great thing about standards is they are so many to chose from...

Think you're right about the si kwarg.

[coveralls]

Coverage increased (+3.0e-05%) to 99.949% when pulling 114ed25 on althonos:patch-1 into 1cbe560 on PyFilesystem:master.

[althonos]

I squashed everything into a single commit, merge shoud be clear now. fs.filesize provides 3 functions, binary, decimal, and traditional.

[willmcgugan]

Great! I hope never to have to think about file size units again :)

[althonos]