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.

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

Incorrect behavior for fetch_nwb in Linearized position tables #912

Closed
samuelbray32 opened this issue Apr 3, 2024 · 5 comments · Fixed by #913
Closed

Incorrect behavior for fetch_nwb in Linearized position tables #912

samuelbray32 opened this issue Apr 3, 2024 · 5 comments · Fixed by #913
Labels
bug Something isn't working position

Comments

@samuelbray32
Copy link
Collaborator

Describe the bug

  • Both LinearizedPositionV1.fetch_nwb() and IntervalLinearizedPosition.fetch_nwb() return a string of the absolute path of the analysis file rather than an open nwb object
  • Causes additional errors within fetch1_dataframe() for these tables

To Reproduce
Both of these return an array of strings rather than open files

from spyglass.linearization.v1 import LinearizedPositionV1
key2 = LinearizedPositionV1().fetch("KEY")[0]
(LinearizedPositionV1() & key2).fetch_nwb()
from spyglass.linearization.v0 import IntervalLinearizedPosition
key2 = IntervalLinearizedPosition().fetch("KEY")[0]
(IntervalLinearizedPosition() & key2).fetch_nwb()
@samuelbray32 samuelbray32 added bug Something isn't working position labels Apr 3, 2024
@samuelbray32
Copy link
Collaborator Author

Correction: Is not specific to the Linearized Position pipeline. TrodesPosV1 fails as well. Probably a recent commit issue

@samuelbray32
Copy link
Collaborator Author

samuelbray32 commented Apr 3, 2024

Checking prior versions, error was introduced in #898

@samuelbray32
Copy link
Collaborator Author

samuelbray32 commented Apr 3, 2024

Got it.

Removing this line in #898 caused the later projection in this file to not capture the relevant object id's

Also, not forcing return as dict here causes the returned structure from fetch_nwb to be different. @edeno is there a reason we need that change?

Edit: wrt second point. forcing return dict as true is necessary for current implementations of fetch1_dataframe

@edeno
Copy link
Collaborator

edeno commented Apr 3, 2024

@CBroz1 actually introduced those changes in a different PR. I just copied them over.

@samuelbray32
Copy link
Collaborator Author

👍

I made a pr that gets it working on the master branch again at #913. Once Chris has a chance to check it doesn't affect new features can merge it in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working position
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants