-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
bpo-29960 _random.Random corrupted on exception in setstate(). #1019
Conversation
@bladebryan, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rhettinger, @serhiy-storchaka and @benjaminp to be potential reviewers. |
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.
Seems this is your first contribution. Add your name in Misc/ACKS
.
Lib/test/test_random.py
Outdated
@@ -446,7 +451,7 @@ def test_setstate_middle_arg(self): | |||
state_values[-1] = float('nan') | |||
state = (int(x) for x in state_values) | |||
self.assertRaises(TypeError, self.gen.setstate, (2, state, None)) | |||
|
|||
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.
Is there a space added?
@@ -436,6 +437,10 @@ def test_setstate_middle_arg(self): | |||
self.gen.setstate((2, (1,)*624+(625,), None)) | |||
with self.assertRaises((ValueError, OverflowError)): | |||
self.gen.setstate((2, (1,)*624+(-1,), None)) | |||
# Failed calls to setstate() should not have changed the state. | |||
bits100 = self.gen.getrandbits(100) |
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.
I don't think that the test is correct. Why not generating bits before calling setstate()?
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.
I'm testing that the failed calls to setstate() did not change the state. I save the initial state, then the failed calls happen. Now I want to know if I'm still in the initial state. I don't check for state equality because a generator's state might not support equality test. I test by generating bits, restoring the initial state, and generating what should be the same bits.
I'm not testing the succeeding call to setstate(). There are other test cases for that, so I assume it works.
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.
I confirm that the test is correct.
Misc/NEWS
Outdated
@@ -10,6 +10,9 @@ What's New in Python 3.7.0 alpha 1? | |||
Core and Builtins | |||
----------------- | |||
|
|||
- bpo-29960: Preserve generator state when _random.Random.setstate() |
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.
This entry should be added in the "Library" section rather than "Core and Builtins".
Thank you for your contribution @bladebryan! Do you mind to backport your chages? |
…ythonGH-1019). (cherry picked from commit 9616a82)
…ythonGH-1019). (cherry picked from commit 9616a82)
…ythonGH-1019). (cherry picked from commit 9616a82)
Changes the _random.Random.setstate() function so that if it raises an exception the state of the generator is unchanged.