Conversation
|
We should probably merge #348 first. I based this PR off that one. I assumed Github would do something smart and only show the new changes w.r.t. the last one, but upon further thoughts, I don't know what I expected, really. |
problemtools/verifyproblem.py
Outdated
| return (desc, p.search, lambda text: p.sub(repl, text)) | ||
|
|
||
|
|
||
| rng = random.Random(42) |
There was a problem hiding this comment.
I would prefer not creating a variable rng for this, but instead do random.Random(42).choice(...) down where you use this variable.
If we create rng and multiple modifiers start using it, we lose determinism if they are reordered or removed (probably no big deal, but if the point of this PR is determinism... :)
There was a problem hiding this comment.
We have to be a little careful: doing random.Random(42).choice(...) will make the same choice 200 times. I found that lambdas are expressive enough to do what we want inline!
There was a problem hiding this comment.
Oh, good catch. I didn't think about the context with how it's called.
|
Yeah, github really could do better in dealing with follow-up PR:s. I'll hold off reviewing this until we merge #347 (as I think it's very close to mergeable). |
3662710 to
5cfac90
Compare
|
Uhh... accidentally deleted this branch when cleaning up old branches because I thought it was merged. Whoops. |
|
Closed; see #351 instead |
As a follow-up to #347, make input fuzzing deterministic.