Repository navigation
Conversation
Member
|
@jdotc1, thanks for the PR. It makes sense to me to eliminate @CameronTEllis, do you think users currently rely on saved noise dicts with |
Author
|
The current patch includes a migration example in the public function documentation and a release note. The example shows how to convert NumPy scalar values to native Python values before saving the dictionary. That helps create compatible files; existing files containing NumPy constructor expressions still need to be re-saved. I don't have evidence about how many users rely on those files, so I'll wait for Cameron's guidance on that compatibility decision. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The real-time generator reads a caller-selected noise-parameter file and evaluates its contents with eval. A file treated as data can therefore execute Python with the process privileges. The bundled parameter dictionary does not require execution.
Replace eval with ast.literal_eval and document the literal-only restriction in the code.
Scope and compatibility
NOT drop-in for all saved files: expressions and NumPy-2 representations such as np.float64(...) are intentionally rejected. calc_noise() can produce NumPy scalar values; convert these to native scalar literals when serializing before using this patch. No general safe NumPy-expression parser is attempted. literal_eval also does not impose input-size/depth limits.
Validation
4 new focused tests passed, exercising the actual generate_data loading path with computational simulation boundaries mocked: literal files, literal bytes, NumPy constructor rejection, and rejection of an expression with a synthetic side-effect marker. The source checkout lacked its generated version module, so the runner supplied package-version metadata only. No full simulation or compiled BrainIAK build.
The supplied source-level evidence for the unchanged fix was reused. No full-project suite, model download, or clinical data was used.
Compatibility migration
This intentionally rejects executable expressions, including NumPy scalar constructor representations. The public function documentation shows how to convert scalar parameters with
value.item()before serializing withrepr(). A release-news fragment is included.