Skip to content

Add tracks dataset validator and movement→ethology bbox loader#141

Open
richarddushime wants to merge 6 commits into
neuroinformatics-unit:mainfrom
richarddushime:demoboxMOT
Open

Add tracks dataset validator and movement→ethology bbox loader#141
richarddushime wants to merge 6 commits into
neuroinformatics-unit:mainfrom
richarddushime:demoboxMOT

Conversation

@richarddushime
Copy link
Copy Markdown
Contributor

@richarddushime richarddushime commented Mar 5, 2026

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

We want ethology to support tracked detections (same individual across frames), so we can later add examples (e.g. boxMOT) and other workflows. Right now we only have annotations (per-frame boxes) and detections (per-frame + confidence). This PR adds the data model and one way to get data into it.

What does this PR do?

Adds ValidBboxTracksDataset: a validator that defines what a “tracks” dataset must look like (same structure as detections, but id means track/individual across frames).

Adds ethology.io.tracks.from_movement_bboxes: a function that takes a movement bbox dataset and turns it into an ethology tracks dataset (renames dims, fills missing category/confidence if needed, validates the result).

References

Please reference any existing issues/PRs that relate to this PR.

issue here

How has this PR been tested?

Please explain how any new code has been tested, and how you have ensured that no existing functionality has changed.
Added required tests

Is this a breaking change?

If this PR breaks any existing functionality, please explain how and why.

No. This only adds new code (validator + loader + tests).

Does this PR require an update to the documentation?

If any features have changed, or have been added. Please explain how the
documentation has been updated.

The new module and function will appear in the API docs once the API index is generated (same as other modules).

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.48%. Comparing base (33adaa5) to head (4dcfd48).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #141      +/-   ##
==========================================
+ Coverage   99.45%   99.48%   +0.03%     
==========================================
  Files           8        9       +1     
  Lines         554      588      +34     
==========================================
+ Hits          551      585      +34     
  Misses          3        3              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@richarddushime
Copy link
Copy Markdown
Contributor Author

The docs job runs make linkcheck and fails on these links, which are not in the ignore list:
https://docutils.sourceforge.io/rst.html
https://github.com/neuroinformatics-unit/movement/blob/main/CONTRIBUTING.md
https://scikit-learn.org/stable/modules/cross_validation.html
Can I fix this by adding them to linkcheck_ignore here so the build no longer fails? i can push an extra small PR fix or a commit on top of this PR
Other PRs are hitting the same issue.

@richarddushime richarddushime marked this pull request as ready for review March 6, 2026 00:05
@sfmig
Copy link
Copy Markdown
Member

sfmig commented Mar 6, 2026

Hi @richarddushime,
I just had a quick look, but can only see the docutils.sourceforge link reported as broken in the doc building action - where do you see the other links failing?

We recently had issues with rate limiting too (see here) which should be fixed with this approach, but I think that is not happening here, right?

@richarddushime
Copy link
Copy Markdown
Contributor Author

richarddushime commented Mar 6, 2026

Hi @richarddushime, I just had a quick look, but can only see the docutils.sourceforge link reported as broken in the doc building action - where do you see the other links failing?

We recently had issues with rate limiting too (see here) which should be fixed with this approach, but I think that is not happening here, right?

these 2 extra links are the ones not added and thought if added can fix the issue completely

the rate limiting issue on this link here

and the link in the examples link here

@sfmig
Copy link
Copy Markdown
Member

sfmig commented Mar 6, 2026

Ok so let's add the docutils.sourceforge link to the list of ignored links, and apply the movement fix for the rate limiting issue.

Are you experiencing the rate limiting error locally? I still cannot find it in the CI logs...

Also the scikit-learn link shows as ok for me in the logs here.

What am I missing?

@richarddushime
Copy link
Copy Markdown
Contributor Author

richarddushime commented Mar 6, 2026

Ok so let's add the docutils.sourceforge link to the list of ignored links, and apply the movement fix for the rate limiting issue.

Are you experiencing the rate limiting error locally? I still cannot find it in the CI logs...

Also the scikit-learn link shows as ok for me in the logs here.

What am I missing?

when i first made a PR ready for review i got that and i had to check but now with the new updates
locally pulled the changes and all is well

the needed fix is the docutils.sourceforge link which has now been fixed in this PR here that you are from merging

All looks Good now

@richarddushime
Copy link
Copy Markdown
Contributor Author

richarddushime commented Mar 6, 2026

All Good the PR you are from merging added the link
here

No extra fix needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants