Make Zarr optional for testing

hdmf-dev / hdmf

The Hierarchical Data Modeling Framework

http://hdmf.readthedocs.io

Other

46 stars 24 forks source link

Make Zarr optional for testing #1141

Closed mavaylon1 closed 23 hours ago

mavaylon1 commented 5 days ago

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?

Show how to reproduce the new behavior (can be a bug fix or a new feature)

Checklist

[ ] Did you update CHANGELOG.md with your changes?
[ ] Does the PR clearly describe the problem and the solution?
[ ] Have you reviewed our Contributing Guide?
[ ] Does the PR use "Fix #XXX" notation to tell GitHub to close the relevant issue numbered XXX when the PR is merged?

codecov[bot] commented 5 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.84%. Comparing base (8917eaf) to head (83defba).

Additional details and impacted files

```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mavaylon1 commented 5 days ago

@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?

mavaylon1 commented 3 days ago

@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.

mavaylon1 commented 1 day ago

@stephprince done