Skip to content
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.

Sign up for GitHub

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

Jump to bottom

nwb interface: allow full paths in __getitem__ (2) #364

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

nwb interface: allow full paths in __getitem__ (2) #364

wants to merge 2 commits into from

Conversation

magland
Copy link
Collaborator

@magland magland commented Nov 6, 2024

This replaces #351

@magland magland requested a review from gviejo as a code owner November 6, 2024 21:42
@magland magland mentioned this pull request Nov 6, 2024
Open
@codecov Codecov
Copy link

codecov bot commented Nov 6, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 19.04762% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pynapple/io/interface_nwb.py 19.04% 15 Missing and 2 partials ⚠️
Files with missing lines Coverage Δ
pynapple/io/interface_nwb.py 72.54% <19.04%> (-6.26%) ⬇️

@gviejo gviejo changed the base branch from main to dev November 6, 2024 22:12
gviejo
gviejo requested changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@gviejo gviejo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add a little test for the exception, it is the file tests/test_nwb.py

pynapple/io/interface_nwb.py
obj = self.nwb.objects[obj_id]
if original_key is not None:
if _path_for_nwb_object(obj) != original_key:
raise KeyError(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to write a little tests for this? I am not sure I understand the case where this would happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gviejo gviejo gviejo requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@magland @gviejo