Append References

[mavaylon1]

Motivation

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

This is in conjunction with hdmf-dev/hdmf#1135

We want to support appending a dataset of references.

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?
• Have you checked our Contributing document?
• Have you ensured the PR clearly describes the problem and the solution?
• Is your contribution compliant with our coding style? This can be checked running ruff from the source directory.
• Have you checked to ensure that there aren't other open Pull Requests for the same change?
• 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]

@rly take a look. We need to merge the PR for zarr optional first, then the HDMF ticket related to this, then this. If you also agree with my approach to the paths (Even though it is a little hacky, I can't think of a better way given our conversation over slack), then I can wrap this up with tests tomorrow.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 54.54545% with 10 lines in your changes missing coverage. Please review.

Project coverage is 83.73%. Comparing base (a508d58) to head (c54504b).

Files	Patch %	Lines
src/hdmf_zarr/zarr_utils.py	25.00%	6 Missing ⚠️
src/hdmf_zarr/backend.py	71.42%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #203      +/-   ##
==========================================
- Coverage   86.05%   83.73%   -2.33%     
==========================================
  Files           5        5              
  Lines        1162     1174      +12     
  Branches      287      289       +2     
==========================================
- Hits         1000      983      -17     
- Misses        107      134      +27     
- Partials       55       57       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.11%. Comparing base (2ba6bc5) to head (7c5c0af).

Files	Patch %	Lines
src/hdmf_zarr/backend.py	81.81%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #203      +/-   ##
==========================================
+ Coverage   86.79%   87.11%   +0.31%     
==========================================
  Files           5        5              
  Lines        1159     1172      +13     
  Branches      285      286       +1     
==========================================
+ Hits         1006     1021      +15     
+ Misses        101      100       -1     
+ Partials       52       51       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mavaylon1]

Review Notes:

The ruff ignores are because changing it to "is" returns errors, i.e., the functionality changes for whatever reason. We can update this if we want, but not in this PR and it would be the lowest of priorities.

[rly]

Adding the new check raises this error when calling read_container.baz_data.data[10]:

if object_path is not None:
            try:
                target_zarr_obj = target_zarr_obj[object_path]
            except Exception:
>               raise ValueError("Found bad link to object %s in file %s" % (object_path, source_file))
E               ValueError: Found bad link to object /new in file /Users/rly/Documents/NWB/hdmf-zarr/test_io.zarr

src/hdmf_zarr/backend.py:741: ValueError

@mavaylon1 could you please look into this?

[mavaylon1]

@rly This is done.

[mavaylon1]

@rly I made the changes. The main thing is to take a look at my notes. Your comment "the parent of the new child has not been built" is weird since the prior code said to build the parent. I assumed you meant the parent of the child could not be the root, leaving the path to the child being possibly incorrect if nested. To ensure a proper path to the child for the reference, we need to build the root. Let me know if that is what you meant.

I added a loop to do so. I did not add a nested test. If we really want one we can do so in a followup PR, but I think I want to explore the proxy idea in case that replaces this.