Complete conversion pipeline: code refactoring #10
Conversation
src/cai_lab_to_nwb/zaki_2024/interfaces/miniscope_imaging_interface.py
Outdated
Show resolved
Hide resolved
| nwbfile: NWBFile, | ||
| motion_correction_series: np.ndarray, | ||
| one_photon_series_name: str, | ||
| convert_to_dtype: DtypeType = None, |
There was a problem hiding this comment.
So I can add the motion correction in the Converter once the OnePhotoSeries has been added
There was a problem hiding this comment.
My question was not very clear, I am talking about convert_to_dtype.
There was a problem hiding this comment.
Ah sorry! No we don't need it, I copied and paste it accidentally
There was a problem hiding this comment.
Can you reference the original implementation if you copied it from somewhere else?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah, is it our thing from one of our repos then. Thanks.
| assert ( | ||
| num_frames == motion_correction_series.shape[0] | ||
| ), f"The number of frames for motion correction ({motion_correction_series.shape[0]}) does not match the number of frames ({num_frames}) from the {one_photon_series_name} imaging data." | ||
| xy_translation = TimeSeries( |
There was a problem hiding this comment.
Shouldn't this be a OnePhtonSeries as well?
There was a problem hiding this comment.
No, for the motion correction we have a specific neurodatatype that store the xt shift as a timeseries and link to the original imaging data
There was a problem hiding this comment.
So why are we using TimeSeries and not the specific neurodatatype? I am confused.
There was a problem hiding this comment.
Sorry, I am confused too apparently...
I wanted to add the MotionCorrection container, but then I decided to store just the xy_shif as a TimeSeries to keep it light.
Co-authored-by: Heberto Mayorquin <[email protected]>
…to refactor_conversion_pipeline
| @@ -14,6 +16,10 @@ def __init__(self, file_path: FilePath, verbose: bool = False): | |||
| self.verbose = verbose | |||
| super().__init__(file_path=file_path) | |||
|
|
|||
| def get_timestamps_reference_time(self): | |||
There was a problem hiding this comment.
get_session_start_time to be in line with the other methods? What do you think?
I don't remember this is "means" object already a datetime object? maybe add a return type or docstring
There was a problem hiding this comment.
I wanted to differentiate from the session start time because this would not be the session start time for this conversion. It's just an auxiliary function to get the time reference of the first timestamp of these traces. I will use this to slice the traces once we understand with the author how they want to align this data.
There was a problem hiding this comment.
This is the case for all the other interfaces. We are never sure if they will be the session_start_time, it is only when you use that interface as stand-alone that it should work that way.
OK, but that makes sense. We will probably not generalize this code outside of this conversion.
NB: how to identify global ids needs to be discussed with the author
I will add the conversion script in a follow up PR