Codecov Report

❌ Patch coverage is 93.38235% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.41%. Comparing base (82b117b) to head (425dc68).

@@             Coverage Diff             @@
##              main     #963      +/-   ##
===========================================
- Coverage   100.00%   99.41%   -0.59%     
===========================================
  Files           41       42       +1     
  Lines         2815     3087     +272     
===========================================
+ Hits          2815     3069     +254     
- Misses           0       18      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sample files needed on GIN

Before this PR can be merged we need at least one (ideally two) real aniframe files on GIN for integration tests. I'll generate these from animovemnt so the embedded R metadata serialisation is exercised end-to-end — the current unit tests cover all edge cases but mock the metadata-decoding step which isn't ideal.

Required: 1 × 2D file

The primary integration test file. It should have:

• 2D coordinates (x, y) and a confidence column
• sampling_rate set to a real value (e.g. 30) — the existing test fixture has NaN here, so fps-from-metadata is currently never tested on real data
• source set to a known software name (e.g. "SLEAP" or "DeepLabCut")
• point_of_reference = "top_left" — so the load completes cleanly without triggering the flip warning (that warning path is already covered by a unit test)
• At least 2 individuals and 2 keypoints
• ~10–20 frames — keep it small for fast downloads
• session and trial columns each holding a single value — confirms the single-value-drop path runs on real metadata

Nice to have: 1 × 3D file

A small 3D file (x, y, z) to verify 3D support against real aniframe output. The 3D array-building path is unit-tested synthetically but has never run against a file produced by R.

What you don't need separate files for

The following are all covered by the synthetic unit tests and do not need dedicated GIN files: track → individual rename, polar coordinate rejection, multi-value session/trial error, missing metadata, and the bottom-left origin warning.

So , outstanding things that have cropped up:

• animovement allows users to convert time to pretty much anything, us, ms, s, min, h, etc. (same goes for spatial units), and then the unit is recorded in the metadata. For now, I assume that you would like to just get seconds? The current solution is to have a lookup table and convert on import.
• the reference frame in animovement is almost always bottom_left - I think I'll need to add a metadata field that allows converting back to original top-left (which is what we often get from the trackers themselves) so it can also be converted back to top-left for import into movement.
• there are some warning in _decode_aniframe_metadata from the rdata package that I have silenced (it's just that the package doesn't know the aniframe class (which makes complete sense).
• extra column names: for extra columns, e.g. computed variables such as "speed", should they just be named as-is, or should they be prefixed to avoid namespace clashes? I'm leaning towards as-is - simply because that's what I would expect as a user.
• as mentioned in the comment above, we need some aniframe parquet files on GIN. Once I know exactly what should be tested, I can make a minimal set of them and @niksirbi can upload them?
• and finally, the dependencies - do we add pyarrow as a required dependency?

Thanks for the PR @roaldarbol! So excited that this is finally happening.

I'll do a first high-level pass at this PR next week, after which I'll get in touch with you to get sample data files.
After I put them on GIN, you can write some tests using them, and I can have another review pass after that.

Sounds like a plan?

Sounds good to me!

For extra columns, I have made the function automatically find the minimal number of dimensions needed to describe it so it automatically gets the correct dims (e.g. if you have the area of the individual, then in animovement all keypoints will have the identical value - and when imported into movement, it would resolve to be a time, individual variable.

The edge case is for single individual/single keypoint cases where extra variables would resolve to just time - to ensure that extra columns be attributes to the correct dimensions I just added an extra argument (extra_var_dims) that allows the user to override it and e.g. say extra_var_dims = "individual" if they should all be that. It also allows specification on a per variable basis:

extra_var_dims={
                      "temperature": ("time",),
                      "bbox_area":   ("time", "individuals"),
                      "speed":       ("time", "individuals"),
                  })

Thanks for such a detailed review @niksirbi! Will go through it later this week!

Before I do anything else, I'm trying to look for lightweight alternatives to pyarrow as it would be preferable to keep the deps obligatory if at all possible - would something like fastparquet, which only also adds ramjam + fsspec, be small enough? Other alternatives are duckdb or polars.

Edit: From the fastparquret docs:

March 2026. The release of pandas 3.0 has broken a number of things in fastparquet. Since pandas now depends explicitly on pyarrow, there is no longer any demand for the existence of this project, and it is being retired. Perhaps use will continue for those still using pandas 2.x, but we anticipate no further development.

For the auto-flipping y-axis, I need to implement something - a metadata field - that keeps the y value, ideally the frame height, but otherwise probably the max. y value from the data (I think that's what I currently use in the readers), which would allow converting between bottom_left and top_left on the fly. And then this value would be pulled in in this reader.

Before I do anything else, I'm trying to look for lightweight alternatives to pyarrow as it would be preferable to keep the deps obligatory if at all possible - would something like fastparquet, which only also adds ramjam + fsspec, be small enough? Other alternatives are duckdb or polars.

Thanks for looking into alternatives! I spent some time looking into them as well. I think we can exclude fastparquet based on the note you've posted. Regarding pandas' dependency on pyarrow, an initial read of that note is slightly misleading: pyarrow isn't included among pandas' core dependencies, it's gated behind parquet extras, see the pyproject.toml. But in any case, that fact is encouraging about the stability and trustworthiness of pyarrow; it's the standard go-to choice within the scientific Python ecosystem for reading parquet files, and more established in that context than duckdb or polars.

My reasoning for having it as an optional dependency is that it's heavy-ish and necessary for only a small subset of users (this may change in the future though...). The counter-argument would be to avoid complicating installation instructions.

Before we reach a decision on this, I'd be curious to hear why you think it's preferable to keep the deps obligatory.

For the auto-flipping y-axis, I need to implement something - a metadata field - that keeps the y value, ideally the frame height, but otherwise probably the max. y value from the data (I think that's what I currently use in the readers), which would allow converting between bottom_left and top_left on the fly. And then this value would be pulled in in this reader.

Ah yeah, that makes sense. If you prefer, you're welcome to proceed here without waiting for that, and tackle the auto-flipping in a future PR (opening an issue to avoid forgetting it). Up to you!

Ah yeah, that makes sense. If you prefer, you're welcome to proceed here without waiting for that, and tackle the auto-flipping in a future PR (opening an issue to avoid forgetting it). Up to you!

I think I'll get it done for this PR. My thought is basically, at read time, to encode either (1) the frame height or (2) the max(y) as y_height. Then a simple flipping function new_y = h - y where h is y_height and y is the vector of y values, will convert back and forth between top and bottom origin. I'll implement such a function in animovement, should I also make a function for it in this movement PR, or should I just do it at read time - pro's is that it could be used for changing the reference frame going forwards.

• If I make such a function, where would you like for it to go?
• And do you think y_height sounds like a reasonable metadata field name?

For pyarrow, yeah I also don't think they got it correct - see the discussion of it here: pandas-dev/pandas#54466.

I think my main concern is on the conda-forge side of things, that users need to explicitly add them as extra dependencies. But with no leaner good alternative I think it's just the way it will have to be. :-) In that case, I imagine we should add an informative error, something like:

try:
    import pyarrow.parquet as pq
except ImportError as e:
    raise ImportError(
        "Reading Parquet files requires the optional 'aniframe' "
        "dependencies (pyarrow, rdata), which are not installed.\n\n"
        "Install them with one of:\n"
        "  pip install 'movement[aniframe]'\n"
        "  conda install -c conda-forge pyarrow rdata\n"
        "  pixi add pyarrow rdata\n\n"
        "See https://movement.neuroinformatics.dev/latest/user_guide/installation.html"
        "for details."
    ) from e

EDIT: I actually quite like the tabbed install instructions, so I don't think it'll be an issue, maybe we'd just need to add it there too? https://movement.neuroinformatics.dev/latest/user_guide/installation.html#install-the-package

I think I'll get it done for this PR. My thought is basically, at read time, to encode either (1) the frame height or (2) the max(y) as y_height. Then a simple flipping function new_y = h - y where h is y_height and y is the vector of y values, will convert back and forth between top and bottom origin. I'll implement such a function in animovement, should I also make a function for it in this movement PR, or should I just do it at read time - pro's is that it could be used for changing the reference frame going forwards.

• If I make such a function, where would you like for it to go?
• And do you think y_height sounds like a reasonable metadata field name?

Sounds sensible. For this PR, I would just implement this at .parquet read time
There's also an argument for making this a standalone utility inside movement.transforms, but it will be easier to think about that once we have the first (read-time) implementation here.

I think my main concern is on the conda-forge side of things, that users need to explicitly add them as extra dependencies. But with no leaner good alternative I think it's just the way it will have to be. :-) In that case, I imagine we should add an informative error, something like:

In the call today, we decided to have pyarrow as extras for now (we may 'promote' it to core dependency in the future).

I think raising an informative error, like the one you suggested, is a great idea.
And yes, I think we should mention this 'extra' in the tabbed installation instructions (as we do for napari).

Alright, now I've spent a few hours going through the comments and addressing them. Here's a few things I think are worth flagging:

• In aniframe I renamed point_of_reference to origin, so I renamed this everywhere (metadata key, ds.attrs, docstrings, tests) to fit.
• I removed extra_var_dims completely and replaced with the singleton-canonical-dim auto-expansion you suggested.
• Now auto-flips y unconditional when origin == "bottom_left". Uses y_height from the metadata when present, falls back to max(y) from the data otherwise, which is how I've also implemented it in aniframe. After flipping, ds.attrs["origin"] is set to top_left and y_height is recorded so the flip can be reversed.
• I also added the dependencies as extras in pyproject.toml, do please have a look to see whether it matches the style you'd expect.
• _parquet_validator is now a generic factory that mirrors _hdf5_validator. The aniframe-specific @file.validator method on ValidAniframeParquet no longer touches pq directly, but delegates the b"r" key check to _decode_aniframe_metadata, then does the required-fields check on top.

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Additional point that we should discuss:

• aniframes can contain multiple trials or sessions or other such temporal variables (variables_when). How should movement handle that, if at all? I wonder whether we automatically could return a list of xarrays? Or should it fail?

Thanks for the updates @roaldarbol. Have been busy with other projects this week but I will have another pass on this next week latest. I'll also think about the sessions/trials question.

Btw, it's looking like #973 is going to be merged ahead of this one, so you will be able to do away with the singular-plural conversion in this PR.

Follow-up for the when issue.

• If there are more than one video (so multiple values in when variables), then we give an error.
• We can supply a parameter (let's session_keys, very not decided) that takes a dict with the name of the variable and the value, e.g. {session: 1, trial: 3}.
• For the error message, provide the names of the when variables - and maybe also potential values?
• Otherwise encourage users to split up on write from aniframe (needs its own issue on aniread).