catalystneuro / nwb-conversion-tools

Create NWB files by converting and combining neural data in proprietary formats and adding essential metadata.

https://nwb-conversion-tools.readthedocs.io/en/main/

BSD 3-Clause "New" or "Revised" License

25 stars 12 forks source link

Add timestamps for MovieDataInterface #527

Closed weiglszonja closed 2 years ago

weiglszonja commented 2 years ago

Motivation

MovieDataInterface currently does not support creating an ImageSeries object with predefined timestamps. The aim of this PR is to run conversion with the capability of supplying a list of timestamps for the movie. Based on: catalystneuro/fee-lab-to-nwb#7

How to test the behavior?

There is a new test created for having timestamps:

test_movie_interface.TestMovieInterface.test_movie_timestamps

Checklist

[x] Have you checked our Contributing document?
[x] Have you ensured the PR description clearly describes the problem and solutions?
[x] Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
[x] If this PR fixes an issue, is the first line of the PR description fix #XXX where XXX is the issue number?

h-mayorquin commented 2 years ago

Not directly related to this PR as it is solving a very specific issue but I feel that this function already has too much cylomatic complexity (lots of loops and ifs) and we are adding a bit more here. At some point, some refactoring to simplify this should be done, maybe having a write_image_series in spikeinterface for one image and loop over there.

CodyCBakerPhD commented 2 years ago

Not directly related to this PR as it is solving a very specific issue but I feel that this function already has too much cylomatic complexity (lots of loops and ifs) and we are adding a bit more here. At some point, some refactoring to simplify this should be done, maybe having a write_image_series in spikeinterface for one image and loop over there.

Sure, low priority, though.

codecov[bot] commented 2 years ago

Codecov Report

Merging #527 (4d31636) into main (e18b378) will increase coverage by 0.03%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #527      +/-   ##
==========================================
+ Coverage   87.74%   87.78%   +0.03%     
==========================================
  Files          58       58              
  Lines        3004     3013       +9     
==========================================
+ Hits         2636     2645       +9     
  Misses        368      368

Flag	Coverage Δ
unittests	`87.78% <100.00%> (+0.03%)`	:arrow_up:

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

Impacted Files	Coverage Δ
...atainterfaces/behavior/movie/moviedatainterface.py	`97.27% <100.00%> (+0.12%)`	:arrow_up:
src/nwb_conversion_tools/utils/__init__.py	`100.00% <100.00%> (ø)`
src/nwb_conversion_tools/utils/checks.py	`100.00% <100.00%> (ø)`