Fix 1-based index translation for pop_interp and pop_editset#251
Open
google-labs-jules[bot] wants to merge 1 commit into
Open
Conversation
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.
This PR addresses critical off-by-one errors in the EEG interactive console when users provide MATLAB-style (1-based) indices. Previously, the system relied on fragile regex patterns to intercept commands, which frequently failed to capture complex user inputs or specific keyword arguments, leading to silent data corruption.
Rationale
To ensure data integrity, we are migrating index translation from simple string pattern matching to a robust AST (Abstract Syntax Tree) transformation layer. This allows the console to accurately identify and decrement integer literals and lists regardless of whether they are provided as positional or keyword arguments.
Key Changes
_POP_INTERP_CHANNELS_PATTERNand_pythonize_pop_interp_channelslogic. These have been replaced by hooks in the_ConsoleCommandArgumentConverterpass.pop_interpSupport:channels,bad_chans, andbad_elec.pop_editsetSupport:icachansindargument.icachansind=15) and positional string sequences (e.g.,'icachansind', 15), converting them to 0-based Python indices.np.arange) remain out of scope to avoid side effects.Success Criteria
pop_interp(EEG, [1, 2, 3])correctly target Python indices[0, 1, 2].bad_chans=[10]) are successfully zero-indexed.icachansindparameter inpop_editsetis correctly translated.