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

fix: handle seed initialization correctly #3005

Closed
wants to merge 1 commit into from

Conversation

rishav2404
Copy link

The previous implementation used a direct property check (config.seed), which could inadvertently treat falsy values like "0" as missing. This caused incorrect random seed initialization in some cases.

Changed the condition to explicitly check for the presence of the seed property using Object.prototype.hasOwnProperty.call(). Now, seed initialization handles falsy but valid seed values correctly.

Resolves #2952.

Description

What is the purpose of this pull request?

This pull request:

  • Bug fix : This pull request addresses the bug fix where setting seed=0 leads to
    non-deterministic behavior.

Related Issues

Does this pull request have any related issues?

No.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

The previous implementation used a direct property check (`config.seed`),
which could inadvertently treat falsy values like "0" as missing. This
caused incorrect random seed initialization in some cases.

Changed the condition to explicitly check for the presence of the `seed`
property using `Object.prototype.hasOwnProperty.call()`. Now, seed
initialization handles falsy but valid seed values correctly.
Copy link
Contributor

@stdlib-bot stdlib-bot left a comment

Choose a reason for hiding this comment

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

👋 Hi there! 👋

And thank you for opening your first pull request! We will review it shortly. 🏃 💨

Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

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

@rishav2404 Thanks for this PR! The bug fix is alright, but could you please use
var hasOwnProp = require( '@stdlib/assert/has-own-property' ); instead of the prototype method here?

@rishav2404
Copy link
Author

@Planeshifter thanks for your guide. I am pushing it in a while.

@kgryte
Copy link
Member

kgryte commented Oct 14, 2024

This PR is superseded by #3007.

@kgryte kgryte closed this Oct 14, 2024
@kgryte kgryte added the Duplicate This issue or pull request already exists. label Oct 14, 2024
@rishav2404 rishav2404 deleted the bug-2952 branch October 14, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate This issue or pull request already exists.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting seed=0 leads to non-deterministic behavior
4 participants