Deal with cells falling outside of the registered atlas boundary #71
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 81.39% 80.47% -0.93%
==========================================
Files 32 32
Lines 1586 1593 +7
==========================================
- Hits 1291 1282 -9
- Misses 295 311 +16 ☔ View full report in Codecov by Sentry. |
| ) | ||
| ) | ||
| ) | ||
| except IndexError: | ||
| if point not in points_out_of_bounds: |
There was a problem hiding this comment.
Just for my understanding: why is this if statement needed? Can there be duplicates in downsampled_points?
There was a problem hiding this comment.
To reduce memory usage, the code above transforms each point in x,y,z separately (otherwise 3 deformation fields need to be loaded at the same time). Without this if, each point would be potentially added to the list three times.
Also I guess there could theoretically be duplicates if someone has edited the results manually.
| transformed_cells, | ||
| atlas.resolution[0], | ||
| brainrender_points_path, | ||
| abc4d_points_path, |
There was a problem hiding this comment.
Maybe worth mentioning in the PR description or in a comment that we remove the abc4d_points_path thing here (not sure what it is, presumably something obsolete?) with a brief justification, so it's clear if we ever come back here?
There was a problem hiding this comment.
I think that's adequately covered by #69
There was a problem hiding this comment.
Cool, yes, I should review PRs in the right order 😅
There was a problem hiding this comment.
I think this is inevitable as I spam the org with 10 PRs a day referencing decisions made 4 years ago!
Description
Deals with the rare case in which a detected cell falls outside of the area in space for which a raw data to atlas space transform exists. This likely happens due to incorrect classification of an artefact outside the brain, or poor quality registration.
Why is this PR needed?
Adds a try/except to skip these cells, and also reports them to the log.
References
Closes #42
How has this PR been tested?
This has been tested locally by running brainmapper and manually adding some cell candidates with coordinates far outside the brain. There are no automated tests added because there are no tests yet for any of this type of code, and it's out of scope for this PR. I've raised #70 to track this.
Is this a breaking change?
No
Does this PR require an update to the documentation?
No
Checklist: