Implement binary prefixes #70

runarberg · 2019-05-06T18:50:03Z

Closes: #33

runarberg · 2019-05-06T19:04:18Z

There are a few considerations:

Is the B name adequate for this type or is there perhaps a better one?
1001Ki is a possible output with enough precision format(".4B") (as opposed to 0.98Mi with format(.3B); implemented here). Is this desirable behavior. Personally I find it confusing, but this is how e.g. human-format does it.
Should I append the tests to the tests/format-type-b-test.js rather then creating the confusingly named tests/format-type-bi-test.js?

I tried to remain consistent in style with the rest of the code base, however I’m not used to writing this terse code so pardon my mistakes 😉

Finally, @mbostock thanks for considering this PR. I’m big fan of your work 😎

haf · 2019-07-12T10:29:43Z

Nice PR; I hope it gets reviewed! @mbostock

mbostock · 2019-07-25T14:13:31Z

src/formatBinaryPrefixAuto.js

+export default function(x, p) {
+  var binaryExponent = 0;
+
+  while (Math.round(x) >= 1024 && binaryExponent < 80) {


Is there a closed form for doing this, rather than needing a loop?

Also it looks like this will hang if you try to format Infinity, but I didn’t test it.

I didn’t investigate any closed forms. I wouldn’t be surprised if one existed using the power of 2. But this loop will run at most 8 times as far as I can tell, even for Infinity.

I added a test that confirms that infinity does not hang indefinitely. However if someone can point out a closed form, it would probably be preferable.

That sounds right; I was too hasty reading the code. Thanks.

Wouldn't it be possible to use Math.log2 here?

(or the IE-compatible equivalent Math.log(number) / Math.log(2))

Closes: d3#33

src/formatBinaryPrefixAuto.js

test/format-type-bi-test.js

Co-authored-by: Philippe Rivière <[email protected]>

curran · 2020-06-18T18:28:47Z

src/formatBinaryPrefixAuto.js

+
+export default function(x, p) {
+  var binaryExponent = 0;
+  if (x === Infinity) return binaryPrefixExponent = 0, x;


This line is confusing.

Why not

if (x === Infinity) return Infinity;

?

Presumably because it needs to set the exported binaryPrefixExponent (note, not binaryExponent).

Aaaahhhhh I see. Side effects. Thanks for pointing that out 🙏

curran · 2020-06-18T18:31:36Z

test/format-type-bi-test.js

+var tape = require("tape"),
+    format = require("../");
+
+tape("format(\"B\") outputs binary-prefix notation with default precision 6", function(test) {


Great comprehensive tests!

src/locale.js

curran · 2020-07-08T14:49:39Z

Suggest to close this in favor of #96

Fil · 2020-07-10T16:41:56Z

closed in favor of #96

runarberg · 2020-11-11T17:32:36Z

Should this be reopened since #96 is still being discussed?

grotlue · 2023-08-21T11:05:30Z

@Fil @curran Hey 👋 Is there any chance this issue will be reopened, as #96 seems to be blocked by the currency discussions?
I would really love this prefix, using d3 for Plotly graphs displaying traffic.

curran · 2023-08-21T14:45:13Z

It looks like that PR #96 is stalled. It has a ton of changes in it. I don't remember the context for all this, but it seems that the PR has grown too large to ever merge. I feel like we need to take a step back and consider the use case(s) and a smaller subset of changes to consider.

@grotlue Is there a particular subset of the changes that interest you? The tests are great - https://github.com/d3/d3-format/pull/96/files - maybe you could identify specific tests that are less ... controversial than all of them together? Then maybe a new smaller PR could be opened that only adds a small part of the feature that would be sensible to add to the library and maintain in the long term, and would be of value to a lot of people.

The other alternative is to author a new NPM package separate from mainline D3 that implements the features from that PR.

runarberg · 2023-08-22T23:00:37Z

I don’t understand why d3-format needs a currency format since Intl.NumberFormat has a pretty good currency formatting and along with { notation: "compact" } gives a strict superset of the benefits of the currency portion of #96 (Intl.NumberFormat also much better localization, and supports much more currencies).

const usdFormat = new Intl.NumberFormat(
  "en-US",
  {
    style: "currency",
    currency: "USD",
    currencyDisplay: "symbol",
    notation: "compact",
  }
);

usdFormat.format(1_000_000)
// => "$1M"

However there is no vanilla way to get binary prefixes (or any SI prefixes) with Intl.NumberFormat. You can use e.g. { style: "unit", unit: "kilobyte" }, however you have to round the number your self before passing it into the format, and the prefixes provided are all SI-based (not binary; so kB as opposed to KiB).

That said, since this PR was closed, I simply rolled my own way of achieving binary prefixes in my own code. #96 (comment)

grotlue · 2023-09-19T15:17:57Z

Hi @runarberg, @Fil, @curran,

thanks a lot for your replies. I had a look at it again and the changes @runarberg mentioned in #33 (comment) and updated in runarberg@3f2383d would be sufficient for my use case. Anything preventing from merging them?

grotlue · 2024-07-18T08:18:05Z

Hi @runarberg @Fil @curran, can this be merged?

mbostock reviewed Jul 25, 2019

View reviewed changes

mbostock mentioned this pull request Jul 25, 2019

Allow SI prefixes to be set in locale. #43

Closed

runarberg force-pushed the master branch from eebecf0 to e5f914e Compare July 25, 2019 22:32

runarberg force-pushed the master branch from e5f914e to 4e982f2 Compare September 2, 2019 20:25

Implement binary prefixes

1dfc9ad

Closes: d3#33

runarberg force-pushed the master branch from 4e982f2 to 1dfc9ad Compare March 28, 2020 00:31

Fil requested changes Jun 4, 2020

View reviewed changes

src/formatBinaryPrefixAuto.js Outdated Show resolved Hide resolved

src/formatBinaryPrefixAuto.js Show resolved Hide resolved

test/format-type-bi-test.js Outdated Show resolved Hide resolved

runarberg and others added 4 commits June 4, 2020 10:16

Update src/formatBinaryPrefixAuto.js

1841fbc

Co-authored-by: Philippe Rivière <[email protected]>

Update src/formatBinaryPrefixAuto.js

b43ed9e

Co-authored-by: Philippe Rivière <[email protected]>

Update test/format-type-bi-test.js

2fb8dbd

Co-authored-by: Philippe Rivière <[email protected]>

Merge branch 'master' into master

e33a8f1

Fil approved these changes Jun 4, 2020

View reviewed changes

curran reviewed Jun 18, 2020

View reviewed changes

src/locale.js Show resolved Hide resolved

Fil mentioned this pull request Jul 7, 2020

Implement Binary prefix, Currency prefix (Merge #70 #81) #96

Closed

Fil closed this Jul 10, 2020

Fil mentioned this pull request Aug 23, 2023

Binary prefixes #33

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement binary prefixes #70

Implement binary prefixes #70

runarberg commented May 6, 2019

runarberg commented May 6, 2019 •

edited

Loading

haf commented Jul 12, 2019

mbostock Jul 25, 2019

mbostock Jul 25, 2019 •

edited

Loading

runarberg Jul 25, 2019

runarberg Jul 25, 2019

mbostock Jul 26, 2019

curran Jun 18, 2020

curran Jun 18, 2020

mbostock Jun 18, 2020

curran Jun 18, 2020

curran Jun 18, 2020

curran commented Jul 8, 2020

Fil commented Jul 10, 2020

runarberg commented Nov 11, 2020

grotlue commented Aug 21, 2023 •

edited

Loading

curran commented Aug 21, 2023

runarberg commented Aug 22, 2023 •

edited

Loading

grotlue commented Sep 19, 2023

grotlue commented Jul 18, 2024

Implement binary prefixes #70

Implement binary prefixes #70

Conversation

runarberg commented May 6, 2019

runarberg commented May 6, 2019 • edited Loading

haf commented Jul 12, 2019

Choose a reason for hiding this comment

mbostock Jul 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

curran commented Jul 8, 2020

Fil commented Jul 10, 2020

runarberg commented Nov 11, 2020

grotlue commented Aug 21, 2023 • edited Loading

curran commented Aug 21, 2023

runarberg commented Aug 22, 2023 • edited Loading

grotlue commented Sep 19, 2023

grotlue commented Jul 18, 2024

runarberg commented May 6, 2019 •

edited

Loading

mbostock Jul 25, 2019 •

edited

Loading

grotlue commented Aug 21, 2023 •

edited

Loading

runarberg commented Aug 22, 2023 •

edited

Loading