Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Byte size to follow IEEE standard #657

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Alexander882
Copy link

Changed ByteSize to follow IEEE and IEC standards for byte sizes (kibi vs kilo, etc.) see #592

@MehdiK
Copy link
Member

MehdiK commented Nov 3, 2017

Thanks for the great contribution. This is a significant breaking change though, and like @omar mentioned here, while technically wrong, is what most people expect.

Can I suggest that you keep the existing functionality as default behavior, to avoid breaking the behavior, and add a strategy to switch to IEEE standard through config? This is an example of a similar implementation.

@dnfclas
Copy link

dnfclas commented May 3, 2018

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ Alexander882 sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@clairernovotny clairernovotny changed the base branch from dev to master May 3, 2018 19:39
@glen-84
Copy link

glen-84 commented Apr 14, 2023

@clairernovotny If the conflicts were resolved, would this PR be accepted?

@SimonCropp
Copy link
Collaborator

@glen-84 i think we should go with the approach suggested by @MehdiK

Can I suggest that you keep the existing functionality as default behavior, to avoid breaking the behavior, and add a strategy to switch to IEEE standard through config

@glen-84
Copy link

glen-84 commented Feb 16, 2024

@SimonCropp

  • The IEC Standard has been adopted by the IEEE, EU, ISO, and NIST, since 2009.
  • Kilo- literally means "thousand", Mega- is used in many places to refer to 1 million, etc.
  • If it's 1,024 bytes by default, then both kilo- and kibi- would be 1,024 bytes.
  • The ByteSize library (authored by Omar, referenced above) switched to SI units.
  • It should mean one thing. When you ask someone how many bytes in a megabyte, they shouldn't answer "it depends".

IMO, the default should be decimal units (1 kB = 1,000 bytes).

@SimonCropp
Copy link
Collaborator

@MehdiK so @glen-84 makes some compelling points. especially since the ByteSize lib has switched over. thoughts?

@MehdiK
Copy link
Member

MehdiK commented Feb 17, 2024

That is a fair point and I completely agree but there are folks who have taken dependency on this - changing the behaviour will break many apps. If we want to change the behaviour, we need to bump the major version. I still suggest we have a strategy in there that allows people to revert back to the old behaviour through config. Thoughts?

@glen-84
Copy link

glen-84 commented Feb 18, 2024

If we want to change the behaviour, we need to bump the major version.

Absolutely.

I still suggest we have a strategy in there that allows people to revert back to the old behaviour through config. Thoughts?

I have no issue with that. I just believe that the default should be decimal units.

@SimonCropp
Copy link
Collaborator

I still suggest we have a strategy in there that allows people to revert back to the old behaviour through config. Thoughts?

@MehdiK i think it would suffice to tell people to move to the new extensions methods. eg "if you want the old behavior change usages of Kilobytes to Kibibytes"

@glen-84
Copy link

glen-84 commented Feb 19, 2024

if you want the old behavior change usages of Kilobytes to Kibibytes

They may still want it to show "kilobytes" and not "kibibytes" in certain cases:

The binary interpretation of metric prefixes is still prominently used by the Microsoft Windows operating system.[9] Binary interpretation is also used for random-access memory capacity, such as main memory and CPU cache size, due to the prevalent binary addressing of memory.

@SimonCropp
Copy link
Collaborator

@glen-84

They may still want it to show "kilobytes" and not "kibibytes" in certain cases:

and they can. there are two extension methods for each unit. take a look at this pr #1413

@glen-84
Copy link

glen-84 commented Feb 19, 2024

Are GetLargestWholeNumberSymbol and GetLargestWholeNumberFullWord even handled in that PR? I think 1,024 bytes will still return KB.

If the code is (1024).Kilobytes().Humanize(), how would you know whether to output 1 MB or 1 MiB, without an option?

@CollinAlpert
Copy link
Contributor

I think 1,024 bytes will still return KB.

@glen-84 That is correct and I think this should be the default anyway. If you want a KiB output, you can specify so in the format by passing "KiB" to the call.

@glen-84
Copy link

glen-84 commented Feb 19, 2024

But what does this output?

var maxFileSize = (10).Kibibytes();

maxFileSize.LargestWholeNumberSymbol; // ?

I'd expect to see KiB, but I think it'll be KB?

@CollinAlpert
Copy link
Contributor

@glen-84 I agree this would be the expectation at first glance. However a more consistent approach would be not to think of kilobytes and kibibytes as separate units, but more of different representations of the actual unit (byte). The same amount of bytes can be expressed in KB as well as KiB. These are not different units.

In your example, I personally would expect "KB" to be printed, since it is more common and many people don't even know what a kibibyte is. For those people who want to have "KiB" displayed, they can still request it.

Also, the implementation would become quite bloated when trying to guess what the developer wants. Say, you write the following: (10).Kibibytes() + (10).Kilobytes()? Does it still make sense to have LargestWholeNumberSymbol print "KiB"?

@glen-84
Copy link

glen-84 commented Feb 20, 2024

If I read:

(1024).Kibibytes().ToString();

I expect to see 1 MiB, not 1.024 MB. If I wanted to see MBs, I would have used (1024).Kilobytes().ToString();.

If the ByteSize took an argument that specified which unit system to use, then you'd be able to use that in GetLargestWholeNumber*.

When adding two ByteSize objects together, the unit system of the first object could be used (binary, in your example above).

This is the way that I see it, but I'm curious as to what others think.

@clairernovotny clairernovotny added this to the v3.0 milestone Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants