githubocto / flat-viewer

Data viewer for Flat repos
https://flatgithub.com
MIT License
137 stars 16 forks source link

Handle fields with units as numbers #31

Open tsekityam opened 2 years ago

tsekityam commented 2 years ago

I have set up a repo to collect holdings of ARK funds using flat

When I sort by market value column, I notice that the sorting is done in pure alphabetical sort order and the data is not sorted by the actual value.

Screenshot 2021-10-12 at 3 36 08 PM

Link to the data

Wattenberger commented 2 years ago

The viewer doesn't currently coerce numbers with units to numbers. Would be a great feature to add! In the meantime, you could remove the $ unit for proper sorting

tsekityam commented 2 years ago

Removing the dollar sign is not enough to have a proper sorting.

The viewer will treat the remaining value (e.g. 123,456,789) as a string because of the thousand separator.

We can try sort the shares column, which doesn't have the dollar sign but thousand separator, and we will see the data are sorted in unnatural order. (e.g. "90,000", "9,000", "8", "700,000")

image

Andrewnt219 commented 2 years ago

Hi @Wattenberger, I want to help with this. I guess we can remove all the things that are not numbers from the data keep only delimiters and numbers, parse those to numbers and sort them?

I can test it out if you point me to the file(s) with the related sorting logic.

tsekityam commented 2 years ago

@Andrewnt219 ,

We determine the data type at https://github.com/githubocto/flat-ui/blob/8f742eebc572a2fa34400777e4ebef01a867a66c/src/store.ts#L482-L488.

      const isFiniteNumber = Number.isFinite(+value);
      if (isFiniteNumber) {
        return [
          metric,
          metric.toLowerCase().trim() === 'year' ? 'year' : 'number',
        ];
      }

For the case $123,456.789, Number.isFinite(+"$123,456.789")) is false, so the UI don't treat it as a number. Let say we remove the $ from the string, Number.isFinite(+"123,456.789")) is still false, so UI don't treat it as a number as well. If seems that we can also remove ,, such that Number.isFinite(+"123456.789")) become true. However, it may cause an error because some locale use , as decimal delimiter.

e.g.

let a = 123.345;
let b = a.toLocaleString('de-DE'); // "123,345"
let c = b.replace(',',''); // "123345"
console.log(+c) // 123345

I considered using globalize to parse locale string, however, it will introduce an extra complexity to the UI. I think it is better to let user to post-process the data to handle the locale.

Anyway, I've prepare a set of mock data for you. The mock data includes a few types of locale string (with thousand separator + decimal separator and with thousand separator + decimal separator + currency)

You can try your implementation with them. Good luck.

Andrewnt219 commented 2 years ago

@tsekityam thanks for your insights. I'll try to figure this out.

Andrewnt219 commented 2 years ago

Ah, it's impossible after all. Without a specified locale, "100.99" can be either 100.99 or 10099.

So I guess we can only go with one locale and force users to follow a specific format for numbers? We can also read the language from req headers and use that, but it doesn't guarantee the data is using the same locale.