Refactor generateTSVString to use pure transformations and remove mutation#748
Open
SarthakJagota wants to merge 1 commit intoneurobagel:mainfrom
Open
Conversation
✅ Deploy Preview for neurobagel-query ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors ResultContainer.generateTSVString to build the TSV output via pure array transformations (map/flatMap) instead of mutating an accumulator array, while preserving the existing TSV format and behavior and tightening type safety around subject_data. Flow diagram for the refactored generateTSVString transformation pipelineflowchart TD
A[generateTSVString called with subjectsResponse and buttonIndex] --> B[Compute isFileWithLabels from buttonIndex]
B --> C[Build headers as tab separated string]
C --> D[Iterate with flatMap over subjectsResponse.responses]
D --> E[For each subResp, find datasetMetadata in datasetsResponse.responses]
E --> F[Destructure dataset fields with default values]
F --> G{isProtected?}
G -- Yes --> H[Build single TSV row for protected dataset]
H --> I[Return array containing single row string]
G -- No --> J{Is subResp.subject_data an array?}
J -- No --> K[Return empty array for this subResp]
J -- Yes --> L[Map subject_data to TSV row strings]
L --> M[Return array of row strings for this subResp]
I --> N[flatMap collects all row strings into dataRows]
M --> N
N --> O[Prepend headers to dataRows]
O --> P[Join all rows with newline characters]
P --> Q[Return final TSV string]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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
This PR refactors generateTSVString to remove mutation-based accumulation and adopt a functional transformation approach.
Changes
Replaced forEach + push with flatMap/map
Removed the TODO comment related to mutation
Eliminated @ts-expect-error by adding proper type narrowing
Preserved the existing TSV output format and column ordering
No behavioral changes
Motivation
The previous implementation relied on mutating an array (tsvRows) inside a loop. This refactor makes the function purely transformation-based, improving clarity and maintainability while maintaining identical output.
Testing
Verified that both labeled and URI TSV downloads produce identical output compared to the previous implementation.
No changes to UI behavior or data formatting.
#747
Summary by Sourcery
Refactor TSV generation in ResultContainer to use a pure transformation-based approach instead of mutating intermediate state.
Enhancements: