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

Refactor nwb extractor frames and tests #136

Merged
merged 7 commits into from
Jun 19, 2022

Conversation

h-mayorquin
Copy link
Collaborator

This is the first attempt at getting correct frames in nwb_extractor. This branch should be tested in nwb-conversion-tools to see if everything works as we expected over there. I refactored the function and added a test to see that it works as expected.

I intend to add a test to support loading TwoPhotonTimeSeries with channels but that's not the priority right now.

@h-mayorquin
Copy link
Collaborator Author

This is the PR in nwb-conversion-tools:

catalystneuro/nwb-conversion-tools#547

@h-mayorquin h-mayorquin self-assigned this Jun 11, 2022
@h-mayorquin h-mayorquin added testing bug Something isn't working high priority labels Jun 11, 2022
@codecov
Copy link

codecov bot commented Jun 18, 2022

Codecov Report

Merging #136 (7903b2c) into master (a6c08b8) will increase coverage by 1.67%.
The diff coverage is 86.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
+ Coverage   66.38%   68.06%   +1.67%     
==========================================
  Files          31       31              
  Lines        1919     1926       +7     
==========================================
+ Hits         1274     1311      +37     
+ Misses        645      615      -30     
Flag Coverage Δ
unittests 68.06% <86.04%> (+1.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...tractors/extractors/nwbextractors/nwbextractors.py 47.73% <81.81%> (+16.10%) ⬆️
src/roiextractors/testing.py 100.00% <100.00%> (ø)

@h-mayorquin
Copy link
Collaborator Author

@CodyCBakerPhD
I corrected to take into account the transposition and the fact that the TwoPhotonSeries does not really have channels as per the discussion in:
NeurodataWithoutBorders/nwb-schema#522

Also, I realized that most of the other get_frames functions do not behave like numpy when indexed with a singleton. I will open another issue for that.

@CodyCBakerPhD CodyCBakerPhD merged commit 984635e into master Jun 19, 2022
@CodyCBakerPhD CodyCBakerPhD deleted the fix_nwb_extractor_frames branch June 19, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants