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

Number utils #3467

Merged
merged 7 commits into from
Apr 30, 2024
Merged

Number utils #3467

merged 7 commits into from
Apr 30, 2024

Conversation

blanchco
Copy link
Contributor

@blanchco blanchco commented Apr 30, 2024

Description

  1. Created some functions to do some conversions for the new input field:
  • number -> exponential
  • exponential -> number
  • number -> NIST
  • NIST -> number
  • displaying a number (this depends on the length)
  1. I have made the input for all these functions a string, since we can deal with very small or very large numbers it can lead to precision issues if we were to use the Number library to manipulate these inputs

  2. UNIT TESTS!!

@blanchco blanchco self-assigned this Apr 30, 2024
@blanchco blanchco linked an issue Apr 30, 2024 that may be closed by this pull request
2 tasks
@mwdchang
Copy link
Member

mwdchang commented Apr 30, 2024

With regards to point-2, I am a bit confused if you are referring to floating-point precision, or if you are talking about allowing arbitrary precision.

  • If the former, string vs number will not solve anything, the problem is inherently baked into binary representation. Short of using a specialized library we just need to be careful and deal with it. e.g. 0.2 * 0.2 != 0.04
  • if the latter, I don't think we need to worry too much about it. Our problem domains should not have extremely large numbers or exceedingly small numbers.

And to illustrate the number vs string precision problem:

> Number(1000000000000000000000000).toExponential(20)
< '9.99999999999999983223e+23'

So overall it would be good if you elaborate a bit more on the design goals.

@blanchco
Copy link
Contributor Author

@mwdchang I've address these comments and am focusing on formatting rather than parsing.

Change made:
removed cleanNumberString
removed numberToExponential, we can just use toExponential
exponentialToNumber now uses toLocaleString to properly display the full number rather than the custom parsing
numberToNist is relatively unchanged aside from using parseFloat to clean the number input
nistToNumber cleans all whitespaces and then parseFloat rather than the custom parsing

@blanchco blanchco merged commit 68b0421 into main Apr 30, 2024
5 checks passed
@blanchco blanchco deleted the number-utls branch April 30, 2024 21:21
});
});

describe('numberToNist', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need example here and nistToNumber and displayNumber for this rule
image

However, when there are only four digits before or after the decimal marker, we recommend that no space is required and no space should be used

For example:
1234.123 -> 1234.123
1234.1234 -> 1234.1234
11234.123 -> 11 234.123
1234.12344 -> 1 234.123 44
11234.123 -> 11 234.123
11234.1234 -> 11 234.123 4

import { exponentialToNumber, numberToNist, nistToNumber, displayNumber } from '@/utils/number';

describe('number util tests', () => {
describe('exponentialToNumber', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should handle this number with thin spaces and exponentials
e.g. -1 234.567 89e+1 -> -1.234 567 89e+4 -> -12345.6789

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.

Number format utilities
3 participants