-
Notifications
You must be signed in to change notification settings - Fork 374
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(stdlibs): add math/rand #2455
Conversation
cc @leohhhn |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2455 +/- ##
==========================================
+ Coverage 54.68% 54.71% +0.03%
==========================================
Files 583 583
Lines 78503 78499 -4
==========================================
+ Hits 42930 42952 +22
+ Misses 32364 32341 -23
+ Partials 3209 3206 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I just left a couple comments with questions. It looks like some of the tests are failing; updating references from rand.Intn
-> rand.IntN
looks like it will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This reverts commit f547d7d.
This PR ports over to Gno the [math/rand/v2](https://pkg.go.dev/math/rand/v2) Go package. It provides pseudo-random number generation. Notable omissions: - the [`N` generic function](https://pkg.go.dev/math/rand/v2@go1.22.4#N) - The [chacha8 random source](https://pkg.go.dev/math/rand/v2@go1.22.4#ChaCha8) (we can use PCG instead; and chacha8 can be implemented later) This PR requires a version bump to go 1.22 to have math/rand/v2 in the standard libraries; otherwise the Native tests won't work. We can rollback the changes to move it to 1.22 by removing the native math/rand implementation, and only using the stdlibs. BREAKING CHANGE: existing usages of math/rand used Go's math/rand; however, in this new standard library we use its v2, so results will be different. Closes gnolang#1272; See-also gnolang#1267 <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details>
- Revert partially "feat(stdlibs): add math/rand (gnolang#2455) (f547d7d) - Update p/demo/rand and rename to p/demo/entropy --------- Signed-off-by: moul <94029+moul@users.noreply.github.com> Co-authored-by: Marc Vertes <mvertes@free.fr>
- Revert partially "feat(stdlibs): add math/rand (gnolang#2455) (f547d7d) - Update p/demo/rand and rename to p/demo/entropy --------- Signed-off-by: moul <94029+moul@users.noreply.github.com> Co-authored-by: Marc Vertes <mvertes@free.fr>
This PR ports over to Gno the math/rand/v2 Go package. It provides pseudo-random number generation.
Notable omissions:
N
generic functionThis PR requires a version bump to go 1.22 to have math/rand/v2 in the standard libraries; otherwise the Native tests won't work. We can rollback the changes to move it to 1.22 by removing the native math/rand implementation, and only using the stdlibs.
BREAKING CHANGE: existing usages of math/rand used Go's math/rand; however, in this new standard library we use its v2, so results will be different.
Closes #1272; See-also #1267
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description