-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support single z-stack tif file for input #88
Conversation
Thanks for this @matham. Just to let you know we should get back to you with a review next week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! I added a few small comments below to make it clearer that dask or numpy arrays may now be returned.
The test failures you encountered should hopefully have been fixed with #89. Could you pull the latest changes from main into this branch? Hopefully that will then fix the CI.
Merging this PR will require brainglobe/brainglobe-utils#67 and brainglobe/cellfinder#397 to be merged first.
Co-authored-by: Kimberly Meechan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks @matham !
I'll merge this now, and aim to make a release of various recent changes on Monday (Friday afternoon releases are dangerous 😆 )
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #88 +/- ##
=======================================
Coverage 85.69% 85.69%
=======================================
Files 16 16
Lines 839 839
=======================================
Hits 719 719
Misses 120 120 ☔ View full report in Codecov by Sentry. |
benchmark test failure expected as we have not released cellfinder with the required new API yet. Other tests pass. |
This is part of this PR: brainglobe/cellfinder#397 and brainglobe/brainglobe-utils#67.
I had to fix this manually for the tests to pass. But this is an issue with the repo, not these changes:
Then is succeded except of these tests, which I don't think are related to this PR