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

feat(number): add distributor functions #3375

Open
wants to merge 7 commits into
base: next
Choose a base branch
from

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Jan 18, 2025

Implements #1862


Adds a distributor option to faker.number.int and float. The distributor function can be used to influence the distribution of the generated values. E.g. uniformDistributor and exponentialDistributor.


The following table shows the rough distribution of values generated using
exponentialDistributor({ base: x }):

Result Base 0.1 Base 0.5 Base 1 Base 2 Base 10
0.0 - 0.1 4.1% 7.4% 10.0% 13.8% 27.8%
0.1 - 0.2 4.5% 7.8% 10.0% 12.5% 16.9%
0.2 - 0.3 5.0% 8.2% 10.0% 11.5% 12.1%
0.3 - 0.4 5.7% 8.7% 10.0% 10.7% 9.4%
0.4 - 0.5 6.6% 9.3% 10.0% 10.0% 7.8%
0.5 - 0.6 7.8% 9.9% 10.0% 9.3% 6.6%
0.6 - 0.7 9.4% 10.7% 10.0% 8.8% 5.7%
0.7 - 0.8 12.1% 11.5% 10.0% 8.2% 5.0%
0.8 - 0.9 16.9% 12.6% 10.0% 7.8% 4.5%
0.9 - 1.0 27.9% 13.8% 10.0% 7.5% 4.1%

The exponential distribution is achieved by using base ** exponent where exponent is a random float between 0 and 1.
The result is then stretched evenly to fit to min and max.

@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent m: number Something is referring to the number module labels Jan 18, 2025
@ST-DDT ST-DDT added this to the vAnytime milestone Jan 18, 2025
@ST-DDT ST-DDT requested review from a team January 18, 2025 15:01
@ST-DDT ST-DDT self-assigned this Jan 18, 2025
Copy link

netlify bot commented Jan 18, 2025

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit a769fb0
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/67c9df14e646c600082472cb
😎 Deploy Preview https://deploy-preview-3375.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Jan 18, 2025

Codecov Report

Attention: Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.

Project coverage is 99.97%. Comparing base (62486af) to head (a769fb0).

Files with missing lines Patch % Lines
src/distributors/exponential.ts 78.57% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next    #3375      +/-   ##
==========================================
- Coverage   99.97%   99.97%   -0.01%     
==========================================
  Files        2811     2814       +3     
  Lines      217414   217443      +29     
  Branches      949      959      +10     
==========================================
+ Hits       217359   217387      +28     
- Misses         55       56       +1     
Files with missing lines Coverage Δ
src/distributors/distributor.ts 100.00% <100.00%> (ø)
src/distributors/uniform.ts 100.00% <100.00%> (ø)
src/index.ts 100.00% <100.00%> (ø)
src/modules/number/index.ts 100.00% <100.00%> (ø)
src/distributors/exponential.ts 78.57% <78.57%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ST-DDT
Copy link
Member Author

ST-DDT commented Jan 18, 2025

I'm also not sure whether exponentialDistribution is the correct name for the function.

@matthewmayer
Copy link
Contributor

I don't like calling it "exponentialDistribution " because you're not returning a distribution, you are returning a single value from that distribution.

So I'd call it exponentialValue or floatExponential or something similar to emphasise the thing that is returned is a float or a value.

@ST-DDT
Copy link
Member Author

ST-DDT commented Jan 19, 2025

you are returning a single value from that distribution.

I'm not even sure that is the case.

So I'd call it exponentialValue or floatExponential or something similar to emphasise the thing that is returned is a float or a value.

Good idea. I'm not finally sure which term I would like to use but stepping away from calling it exponentialDistribution is the correct choice.

I'm currently considering one of these:

  • exponential
  • exponentialValue
  • exponentialNumber
  • exponentialFloat
  • floatExponential
  • exponentiallySpread (or something similar)
  • powerLaw___ (<- ChatGpt)

@xDivisionByZerox
Copy link
Member

Team decision

The feature will be implemented by extending the already existing methods (number.int(), number.float(), etc). An additional parameter will be added. The parameter will be a function that exepts a randomizer instance and will return a biased randomized value. The return value will be used by the original function to "push" the value in the direction the distribution desires.

We are currently unsure how to name the parameter. While distribution is okay, we are not sure if this is the mathmatical correct term.
The name "spread" was discussed, but discarded since it might confuse users with the JS spread operator.
We will think about other names until the next meeting. If we do not come up with any, the current name of "distribution" is fine for us.

@ST-DDT ST-DDT changed the title feat(number): add exponentialDistribution function feat(number): add distributor functions Feb 17, 2025
@ST-DDT
Copy link
Member Author

ST-DDT commented Feb 17, 2025

Ready for review.

I changed the implementations to use a distributor parameter.
distributor is close enough to distribution so that you can guess what the option does, without actually running into the mathematical specification.

@ST-DDT ST-DDT linked an issue Feb 17, 2025 that may be closed by this pull request
@ST-DDT ST-DDT requested a review from a team February 17, 2025 21:56
Comment on lines +37 to +41
export function uniformDistributor(): Distributor {
return UNIFORM_DISTRIBUTOR;
}

const UNIFORM_DISTRIBUTOR: Distributor = ({ next }) => next();
Copy link
Member

Choose a reason for hiding this comment

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

I thought quite long about it. What exactly was your reasoning to extract the uniform function implementation into it's own function expression? The only benifit I was able to see, would be function reference comparison:

const a = uniformDistributor();
const b = uniformDistributor();
a === b // true

Was that your reasoning? If yes, is this really desired? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature m: number Something is referring to the number module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding Random probability distribution function
4 participants