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

Provide coordinates for both time and frame #117

Open
niksirbi opened this issue Feb 13, 2024 · 6 comments
Open

Provide coordinates for both time and frame #117

niksirbi opened this issue Feb 13, 2024 · 6 comments
Assignees
Labels
enhancement New optional feature

Comments

@niksirbi
Copy link
Member

Is your feature request related to a problem? Please describe.
Currently in our xarray.Dataset we only have one time axis, the units of which may vary depending on whether the user has provided an fps value or not. This is a bit confusing, the coords of that axis may be in seconds or in frames. We keep track of the units in Dataset.attrs, but it may still cause confusion.

Describe the solution you'd like
We could have both coordinates systems -frame and time (in seconds) associated with the same axis/dimension. The user should be able to do ds.sel(frame=10) as well as ds.sel(time=200). AFAIK this is supported by xarray, see example implementation in xarray-video.

Describe alternatives you've considered
Alternatively we could alway require fps and only have the time coords in seconds. But even if we do that, it's still valuable to have both time and frame coords defined, as users might need one or the other, depending on the occasion.

@niksirbi
Copy link
Member Author

What do y'all think @neuroinformatics-unit/behaviour @b-peri ?

@adamltyson
Copy link
Member

Both sounds like a good idea to me.

@b-peri
Copy link
Collaborator

b-peri commented Feb 19, 2024

I agree - having both would be super useful!

@niksirbi niksirbi moved this from 🤔 Triage to ⚠️ Priority in movement progress tracker Feb 22, 2024
@niksirbi
Copy link
Member Author

After discussing at the team meeting today, we settled on the following course of action:

  • frame axis should always be created when loading the data
  • fps is optional, but if the user provides it we also create a time axis in seconds.
  • We also need a way for people to provide the fps later, if they didn't do so during file loading.
  • If the user requests an operation that requires the time axis, but this doesn't exist yet, we should prompt them to supply the fps first (e.g. in the error message).

@talmo
Copy link

talmo commented Feb 23, 2024

An option to provide explicit vector of timestamps for the time axis would cover this common scenario.

Great point, also useful for synchronising with other modalities. We should definitely provide a method for people to pass a vector of timestamps instead of fps.

@niksirbi niksirbi moved this from ⚠️ Priority to 📝 Todo in movement progress tracker Mar 18, 2024
@niksirbi niksirbi moved this from 📝 Todo to ⚠️ Priority in movement progress tracker Mar 25, 2024
@niksirbi niksirbi self-assigned this Mar 25, 2024
@niksirbi
Copy link
Member Author

xarray supports dimensions with multiuple coordinate axes, but we'd have to carefully review the relevant terminilogy of dimension- vs non-dimension coordinates and indexed-vs non-indexed coordinates. Ideally we want both time and frames to be indexed coordinates, i.e. ones that permit slicing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New optional feature
Projects
Status: 🤔 Triage
Development

No branches or pull requests

4 participants