Metadata handling, export, and real-time visualization

[t-sasatani]

Some updates for interfacing metadata and accompanying updates for the CLI.

Changes:

• Patch https://github.com/Aharoni-Lab/miniscope-io/issues/34
• Add metadata CSV export and ~circular buffer queue~ shared list (for use in other processes)
• Add BufferedCSVWriter for buffered CSV exports.
• Change CLI to output files with the same stem names (for -o sample, you get sample.avi, sample.csv, sample.bin)

Notes:

• I used the runtime config module to add some environment variables, but I am unsure if I used it as @sneakers-the-rat intended. I would appreciate feedback.
• Now, the video export is locked to .avi, which is good for now, but it might be better to define this by environment variables or something. There's no point doing this now, so we can probably leave it until we build an interface for video export options.

[coveralls]

coverage: 73.757% (+1.7%) from 72.069% when pulling 479363c47248d87dcb1c276edaecf2c7c81bcef9 on feat-metadata-handle into 22908743959a3a8b0d69a725928d07eb915db249 on main.

[sneakers-the-rat]

Ope sorry missed this when u opened it. I got some notes but like what yr thinkin here :)

[t-sasatani]

No problem. This isn't a hurry at all!

[t-sasatani]

Ok, the video hash seems to change if the queue size changes (I confirmed locally by changing the env variables). This might just mean that we're getting better videos, or it could mean some mystery is happening to video hashes.

[t-sasatani]

Actually, we get more video/metadata when the queues are smaller, so I assume this happens because of the termination order of the processes or something. Will look into this later.

[t-sasatani]

I added a metadata plotting feature because I need them for experiments next week. We might want a neater GUI soon, but this should work as a temporary solution. I included it in this PR because it is closely related, but I'm also happy to push this to a later one.

This feature runs by the following steps:

1. Configure these runtime configs if necessary (specify metadata keys to be plotted).

   MINISCOPE_IO_STREAM_HEADER_PLOT_HISTORY=500
   MINISCOPE_IO_STREAM_HEADER_PLOT_UPDATE_MS=100
   MINISCOPE_IO_STREAM_HEADER_PLOT_KEY="timestamp,buffer_count,linked_list,frame_buffer_count"

2. Use the -m option in mio.
   • https://miniscope-io--35.org.readthedocs.build/en/35/cli/main.html

This shows a real-time plot of metadata.

[sneakers-the-rat]

ok @t-sasatani see my updates in the above commits, tried to split them up so they are a little easier to follow, but they are sorta intrinsically related to each other.

Basically i am trying to have as light of a touch as possible here bc i know y'all are under a deadline while trying to avoid this package getting too spaghettilike from that rush against deadline.

• Move streamdaq runtime config to its own configuration model: since these are parameters that should be generally true for this device, moved them there - the top-level Config object is for behavior that should be true across the entire package. as this package grows we want to avoid putting things at the top level if they are only used in some part of the module. We also want to avoid having too many ways to configure things without a clear structure in place: streamdaq config is already set in those yaml files, so kept it there.
• Move csvwriter out of the inner loops which are perf sensitive to the outer loop which shouldn't be. We'll need a bigger refactor here eventually, but as i said above trying to not touch too much while we're on deadline. those inner methods should be super minimal and only do exactly one thing. the capture method is already a junk drawer method (hence need for refactor) but just trying to keep similar things in similar places for now.
• Make plotter handle its own timing - simplify the API to that class so it's just "give me objects to plot" instead of every object that uses it needing to manage draw timing. I think you might also like some of the other tidying there, hopefully that makes sense.
• Make basic e2e tests for these functions - we'll need a great deal more testing for this whole thing, the entirety of the inner methods are untested atm which i suspect is the source of continual regressions, but at least we can keep up with top-level tests.

There are some test errors that are only showing up in CI on ubuntu that i'm gonna fix, but other than that i'll take one final look and approve

[sneakers-the-rat]

ok here's a fun one for you - for the csv writer test, where since i didn't know exactly what we would want there, i just did a simple shape test, where the header metadata that gets written should be the same every time (for a given test data file), but we are getting...

• linux: 893 rows
• windows: 901 rows
• mac: 901 rows (and, locally, on my machine, getting 855 rows, which is why the value is set that way).

I'm not really sure what that value should be since idk the source data, but the video is 20fps, 5.5s long = 110 frames, and there should be 8 buffers per frame = 880 total, which is actually none of those values.

so... ya...

[sneakers-the-rat]

OK fixed those bugs, and fixed the early termination bugs - removed ability to exit with escape while displaying videos, now there are two ways to quit a capture:

• terminate event is the underlying holder of quitting logic
• ctrl+c causes terminate to get flipped to true.

but instead of prior behavior where all the loops quit immediately when getting the terminate event, refactored the top-level capture frame processing logic into a separate method so it was possible to call in two places, so now we explicitly wait for the queues to drain before fully quitting (it's also possible to force quit with a second ctrl+c).

very very strong need to break apart those inner methods so that we have better access to them for unit testing, bc we are gonna keep getting mystery errors as long as we only have end-to-end tests and can't directly test them as pure functions, given some expected input, get some expected output, separated from the rest of the acquisition process.

[sneakers-the-rat]

Added tests that confirm that varying buffer size doesn't change output to guard against the problems @t-sasatani was describing here: https://github.com/Aharoni-Lab/miniscope-io/pull/35#issuecomment-2293335064 which was because of the termination logic

[sneakers-the-rat]

Down with merging this btw :)

[t-sasatani]

Thanks sounds great. I'm thinking about making some changes to save more messed-up metadata, but I will bump up a minor version after I decide what to do with that.

[sneakers-the-rat]

Merge this then do that?

[t-sasatani]

The refactored version is running very slow compared to https://github.com/Aharoni-Lab/miniscope-io/pull/35/commits/5d33e3b13e30988169e04a9e951c59b65041b147, and I'm looking why. I assume it's blocking/non-blocking stuff in plots, and here is an example binary to see this. https://drive.google.com/open?id=1QoLB1F7HUau6TDrHQWxpivR48RhpTfWN&usp=drive_fs

[t-sasatani]

This was just the window refresh (cv2.waitKey(1)) getting deleted with the ESC key escape. Also, the plot window becomes unstable when moved (changing the GUI backends of matplotlib didn't fix it), but I guess I'll merge it with an experimental notice.

It might be worth considering GUI tests, but I'm unsure if there's a good way to do non-web GUI tests on GitHub actions.