default load_namespace - Githubissues

hdmf-dev / hdmf-zarr

Zarr I/O backend for HDMF

https://hdmf-zarr.readthedocs.io/

Other

7 stars 7 forks source link

default load_namespace #204

Closed mavaylon1 closed 1 week ago

mavaylon1 commented 2 weeks ago

Motivation

What was the reasoning behind this change? Please explain the changes briefly. Fix #198

How to test the behavior?

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

Checklist

[x] Did you update CHANGELOG.md with your changes?
[x] Have you checked our Contributing document?
[x] Have you ensured the PR clearly describes the problem and the solution?
[x] Is your contribution compliant with our coding style? This can be checked running ruff from the source directory.
[x] Have you checked to ensure that there aren't other open Pull Requests for the same change?
[x] Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

mavaylon1 commented 2 weeks ago

@rly Looking at the pynwb version of this (https://github.com/NeurodataWithoutBorders/pynwb/pull/1748/files#) I don't see any tests that tries to read a file with extensions. As a result, there isn't any tests I see that need to be updated. Maybe added, but I'm fine with that being a coverage PR.

codecov-commenter commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 86.54%. Comparing base (a508d58) to head (b175dc9).

Files	Patch %	Lines
src/hdmf_zarr/nwb.py	40.00%	1 Missing and 2 partials :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## dev #204 +/- ## ========================================== + Coverage 86.05% 86.54% +0.48% ========================================== Files 5 5 Lines 1162 1159 -3 Branches 287 285 -2 ========================================== + Hits 1000 1003 +3 + Misses 107 103 -4 + Partials 55 53 -2 ```

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

mavaylon1 commented 2 weeks ago

the ruff error is fixed in a different PR (the zarr append PR)

rly commented 2 weeks ago

This looks good to me. Please add a changelog

mavaylon1 commented 1 week ago

@stephprince already approved by ryan. Just needed a changelog