SilverLabUCL / PySilverLabNWB

Python tools for working with Silver Lab data in the NWB2 format
MIT License
1 stars 0 forks source link

Use a class for LabView headers #33

Closed ageorgou closed 4 years ago

ageorgou commented 4 years ago

This should make it easier to vary the behaviour for different LabView versions, and hide the details away from the main NwbFile class as they're getting more complex.

alessandrofelder commented 4 years ago

Made two small commits that should fix the flake8 failures on Travis 🤞 Am I right in thinking the data that came ImagingInformation now is part of the headers?

codecov-io commented 4 years ago

Codecov Report

Merging #33 into new-labview will increase coverage by 0.87%. The diff coverage is 76.80%.

Impacted file tree graph

@@               Coverage Diff               @@
##           new-labview      #33      +/-   ##
===============================================
+ Coverage        50.62%   51.50%   +0.87%     
===============================================
  Files               12       14       +2     
  Lines             1590     1627      +37     
  Branches           261      258       -3     
===============================================
+ Hits               805      838      +33     
- Misses             751      757       +6     
+ Partials            34       32       -2     
Impacted Files Coverage Δ
src/silverlabnwb/header.py 72.81% <72.81%> (ø)
src/silverlabnwb/nwb_file.py 91.55% <87.50%> (+3.18%) :arrow_up:
src/silverlabnwb/imaging.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 60d3381...ec3143c. Read the comment docs.

ageorgou commented 4 years ago

Am I right in thinking the data that came ImagingInformation now is part of the headers?

All the data that is needed to create the ImagingInformation is now in the header. The code in NwbFile now gets the ImagingInformation object by delegating to the LabViewHeader (calling header.get_imaging_information()), and stores the result (so it can then forget about the header - at least for the time being).

ageorgou commented 4 years ago

Removed the TODO on line 64 which is not needed. For the TODO on casting values later, it may actually be better to overhaul this, since we're not really storing keys and and values, but the two numbers in the line. Perhaps we can store the whole line as a value with the key "line 1" or similar, or even the text of the whole section as a single value. The former seems easier. Then we can break down the line and cast the numbers when computing the trial times. I would suggest putting this in a different, small PR after we have some tests although it could also go here!