Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify terminology for frames in frame_to_time and get_frame. #202

Open
h-mayorquin opened this issue Aug 31, 2022 · 3 comments
Open

Unify terminology for frames in frame_to_time and get_frame. #202

h-mayorquin opened this issue Aug 31, 2022 · 3 comments
Labels

Comments

@h-mayorquin
Copy link
Collaborator

Right now we have frames on the functions for getting time:
https://github.com/catalystneuro/roiextractors/blob/master/src/roiextractors/imagingextractor.py#L69

And frame_idxs in the function for getting the frames:

def get_frames(self, frame_idxs: ArrayType, channel: Optional[int] = 0) -> np.ndarray:
assert max(frame_idxs) <= self.get_num_frames(), "'frame_idxs' exceed number of frames"
if np.all(np.diff(frame_idxs) == 0):
return self.get_video(start_frame=frame_idxs[0], end_frame=frame_idxs[-1])
relative_indices = np.array(frame_idxs) - frame_idxs[0]
return self.get_video(start_frame=frame_idxs[0], end_frame=frame_idxs[-1] + 1)[relative_indices, ..., channel]

But both of them are referring to the same. We should use either frames, frame_idxs or frame_indices for the two of them. Numpy uses indices for their indexing routines so maybe that's a good choice:
https://numpy.org/doc/stable/reference/arrays.indexing.html#routines-indexing

But I don't think it matters that much.

@CodyCBakerPhD
Copy link
Member

I'd vote for frames

@CodyCBakerPhD
Copy link
Member

Although, specific to the usage in frame_to_time, how badly do we want to mimic SpikeInterface and plan to eventually just have a single get_times function that either uses an underlying irregular time_vector or otherwise returns a np.arange scaled by the sampling frequency and shifted by an offset?

https://github.com/SpikeInterface/spikeinterface/blob/master/spikeinterface/core/baserecording.py#L441-L452

@h-mayorquin
Copy link
Collaborator Author

Yeah, maybe going for get_times for consistency is better overall.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants