Repository navigation
Fix correctness issues in readers, dF/F, deconvolution and FluFinder detection - #1
Open
ehennestad wants to merge 3 commits into
Open
ehennestad wants to merge 3 commits into
ehennestad wants to merge 3 commits into
Conversation
…detection - MDF: drop the private copy of resolveDataSizeAndDimensionArrangement, which the VirtualArray base class seals; the class could not be loaded at all, so MScan recordings could not be opened. - SciScanRaw.fileCheck returns a scalar logical. - CaImAn deconvolve: only call gcp when the Parallel Computing Toolbox is installed; it merely decides whether to print progress. - dffRoiMinusDffNpil: clear error when sgolayfilt (Signal Processing Toolbox) is missing, instead of an undefined-function error. - dffChenEtAl2013: take the baseline percentiles along time. - deconvolveDff: the input contract is numSamples x numRois (a vector is one ROI); orientation is no longer guessed from which dimension is larger, and the function now has help text. - FluFinder findUniqueRoisFromComponents: build the component image with accumarray (histcounts needed two bin edges and dropped the last unique pixel), return early when there are no components, and stop when the component image has no peak left instead of looping forever. - extractF help names RoI arrays instead of a nonexistent type. - ThorLabsTiffs and Suite2pCorrected call their own static method by qualified name instead of importing themselves. - Remove the EXTRACT ProcessorT class, unreferenced since 2022 apart from a commented-out block, and that block. Tests: every reader class must load, the SciScan predicate is scalar, the Chen dF/F of one ROI matches the multi-ROI result, and FluFinder's component picking handles no components, one repeated component and two separated components. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The explicit permissions block lacked pull-requests: write, so the publish-unit-test-result step was refused with 403 on the first PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Test Results39 tests 38 ✅ 4s ⏱️ Results for commit 57b1bcd. |
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
1 participant
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
Batch A from the migration memo: small correctness fixes in the algorithm code that moved into this module, each with a test.
Fixes
MDFkept a private copy ofresolveDataSizeAndDimensionArrangement, whichnansen.stack.data.VirtualArraydeclares sealed, someta.class.fromNamefailed and no.mdfrecording could be opened. The copy is removed; the base method does the same thing.SciScanRaw.fileCheckreturned a 1x2 logical fromcontainswith two keys; it now returns a scalar.deconvolvecalledgcpunconditionally to decide whether to print progress; without the Parallel Computing Toolbox that was an undefined-function error before any work started.dffRoiMinusDffNpilnow raises a clear error naming the Signal Processing Toolbox whensgolayfiltis missing, and points at the alternatives.dffChenEtAl2013takes both baseline percentiles along time, asdffClassicdoes, instead of relying on the default dimension.deconvolveDffdocuments and enforces its contract, numSamples x numRois in and out with a vector as one ROI, instead of transposing whenever rows outnumbered columns, which silently deconvolved the wrong axis for recordings with more ROIs than samples. All callers already pass that shape.findUniqueRoisFromComponentsbuilds the component image withaccumarray. Thehistcountsversion needed at least two bin edges and, because the last bin is closed, never counted the last unique pixel. The function returns early when there are no components and stops when the component image has no peak left, which previously spun forever after thecontinuebranch had cleared every pixel.extractFhelp named a nonexistentnansen.RoiArraytype.ThorLabsTiffsandSuite2pCorrectedimported themselves to call a static method; they use the qualified name now.ProcessorTin the EXTRACT integration is deleted together with the commented-out block that was its only reference. It has been unreferenced since 2022, has an emptygetDefaultOptions, and reusedProcessor's options identifier. Easy to restore from history if it turns out to be wanted.Tests
ReaderResolutionTestnow loads every reader class (this would have caught MDF) and checks the SciScan predicate is scalar.DffTestchecks the Chen dF/F of a single ROI matches the multi-ROI result. NewFluFinderDetectionTestcovers no components, one repeated component and two separated components with hand-built stats in the shapegetBwComponentStatsreturns.🤖 Generated with Claude Code