feat(number)!: default to high precision float

[matthewmayer]

fix #1655

Note in order to reduce unexpected changes, i made a new code path for the default case when precision is not passed which grabs a float from Mersenne directly, while keeping the old logic intact (going via an integer) when a precision is pased.

I also added {precision: 0.01} in a few places in the code where float() was previously used to avoid unnecessary changes.

[codecov]

Codecov Report

Merging #1675 (79eda17) into next (0663048) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 79eda17 differs from pull request most recent head b9af667. Consider uploading reports for the commit b9af667 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             next    #1675   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files        2340     2340           
  Lines      234254   234260    +6     
  Branches     1112     1116    +4     
=======================================
+ Hits       233421   233427    +6     
  Misses        812      812           
  Partials       21       21           

Impacted Files	Coverage Δ
src/internal/mersenne/mersenne.ts	100.00% <100.00%> (ø)
src/modules/color/index.ts	99.79% <100.00%> (ø)
src/modules/number/index.ts	100.00% <100.00%> (ø)

[ST-DDT]

Looks like we broke a test.

[matthewmayer]

I don't think that test makes much sense (precision 0)

Probably throwing an error is better

[matthewmayer]

Some weird error with codecov on CI

[ST-DDT]

I don't think that test makes much sense (precision 0)

Probably throwing an error is better

Mhh, a float with a precision of zero is an int.
I would still call it a valid value, but IMO that is not that important.
What do the others think?

[matthewmayer]

I don't think that test makes much sense (precision 0)
Probably throwing an error is better

Mhh, a float with a precision of zero is an int. I would still call it a valid value, but IMO that is not that important. What do the others think?

no, a float with precision 1 is an int

[ST-DDT]

no, a float with precision 1 is an int

Oh yeah, you are totally right.

Do we have a test that ensures the following

const actual = faker.number.float({precision: 1, ?});
expect(actual).toSatisfy(Number.isInteger);

?

[matthewmayer]

i don't think this needs the breaking change label? Because this is only applied to faker.number.float() which is new in 8.0.0, not the old faker.datatype.float()

[xDivisionByZerox]

i don't think this needs the breaking change label? Because this is only applied to faker.number.float() which is new in 8.0.0, not the old faker.datatype.float()

You did call it a breaking change yourself. Even if only in a private function. Please be consistent whether you want to support JS or not! If yes, this is a breaking change for JS users since the way mersenne returns values was changed.

Edit: Thats exactly why I hate mixing points of interest in PRs! This change could have been done without touching mersenne in any way.

[Shinigami92]

i don't think this needs the breaking change label? Because this is only applied to faker.number.float() which is new in 8.0.0, not the old faker.datatype.float()

You did call it a breaking change yourself. Even if only in a private function. Please be consistent whether you want to support JS or not! If yes, this is a breaking change for JS users since the way mersenne returns values was changed.

Edit: Thats exactly why I hate mixing points of interest in PRs! This change could have been done without touching mersenne in any way.

TBH I understand @xDivisionByZerox points here
I think it is good to mark this as breaking change anyway, because it affects a lot of other method calls inside faker like color.rgb.

I also understand that it might sometimes be better to make smaller PRs and split them, but for now I'm fine with the current state (except the english spelling in JSDoc), but then I can approve this 👍

[matthewmayer]

There's not really a good definition of breaking change. You have to use your common sense as every change is a breaking change if your workflow is weird enough!

Anyway since this IS landing in a major release, whether it is breaking or not is moot.

https://xkcd.com/1172/