-
Notifications
You must be signed in to change notification settings - Fork 26
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
Make Zarr optional for testing #1141
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1141 +/- ##
==========================================
+ Coverage 88.78% 88.84% +0.05%
==========================================
Files 45 45
Lines 9782 9786 +4
Branches 2778 2778
==========================================
+ Hits 8685 8694 +9
+ Misses 780 776 -4
+ Partials 317 316 -1 ☔ View full report in Codecov by Sentry. |
@rly We don't currently cover the else statement in append data. The gap in this PR is the same else statment, but under the zarr conditional. Are you okay with this gap in coverage for now? |
@rly made a simple example. It has full coverage and covers the missing spot I mentioned from prior to this PR. It's not a actual iterable class since it does not have iter or next, but I think it is fine. |
Co-authored-by: Steph Prince <[email protected]>
Co-authored-by: Steph Prince <[email protected]>
@stephprince done |
Motivation
What was the reasoning behind this change? Please explain the changes briefly.
After discussions with the team, we decided to have the zarr be an optional dependency.
How to test the behavior?
Checklist
CHANGELOG.md
with your changes?