Add Facemap interface and related dependencies for behavioral data processing #1608
Conversation
for more information, see https://pre-commit.ci
| self, | ||
| nwbfile: NWBFile, | ||
| metadata: Optional[dict] = None, | ||
| compression: Optional[str] = "gzip", |
There was a problem hiding this comment.
We no longer have compression parameters in the add_to_nwbfile methods as the backend configuration takes care of it in a more general and streamlined way and they might clash.
| return self.timestamps | ||
|
|
||
| def set_aligned_timestamps(self, aligned_timestamps: np.ndarray) -> None: | ||
| self.timestamps = aligned_timestamps |
There was a problem hiding this comment.
I think it is better to make self.timestamps private to keep the API of the class small and more flexible.
| Path to the .mat file. | ||
| video_file_path : string or Path | ||
| Path to the .avi file. | ||
| first_n_components : int, default: 500 |
There was a problem hiding this comment.
Is first_n_components like a stub_test or something different? If the former, the convention we have is to make it part of the conversion options in the add_to_nwbfile kwargs.
| from ndx_facemap_motionsvd import MotionSVDMasks, MotionSVDSeries | ||
| except ImportError: | ||
| # TODO: to be change when ndx-facemap-motionsvd version on pip | ||
| install_package("git+https://github.com/catalystneuro/ndx-facemap-motionsvd.git@main") |
There was a problem hiding this comment.
In the other interfaces we just import those things at init and we let the error surface as it is.
I think installing thinks on behalf of the user without a prompt is bad and I don't think the complexity of implement a prompt is worth it.
| "ndx-events==0.2.1", | ||
| ] | ||
| facemap = [ | ||
| "ndx-facemap-motionsvd @ git+https://github.com/catalystneuro/ndx-facemap-motionsvd.git", |
There was a problem hiding this comment.
Note that we can't release to pypi with repo based dependencies. Do you know if there is a timeline for release?
| Compression options. | ||
| """ | ||
| # self.add_eye_tracking(nwbfile=nwbfile, metadata=metadata) | ||
| self.add_pupil_data(nwbfile=nwbfile, metadata=metadata, pupil_trace_type="area_raw") |
There was a problem hiding this comment.
I would prefer to keep the data methods add_pupila_data and add_motion_SVD private. Again to keep this more flexible and the API smaller. Let's make things if and when we need to make them public.
No description provided.