Skip to content

Conversation

@fe51
Copy link
Member

@fe51 fe51 commented Dec 12, 2025

This PR to address #528 which is part of the planned changes to the data model

It introduces the following modifications

  • Add table poses in models and add the fk to the related tables (currently nullable while the migration is being carried out)
  • Add poses schema and updates other related
  • Add poses crud
  • Add pose crud endpoints (GET, POST, PATCH, DELETE)
  • Updates camera endpoints (to reflect cameras poses)
  • Updates detections endpoints (to include poses) -> later, in migration, azimuth will be delete, as camera_id fk (even if camera_id can be still returned by api)
  • Update tests
  • Updates db diagram

What is missing (to be discussed of to handle in another PR)

  • Alembic migrations
  • Client updates -> will be done later
  • Client tests (struggled to be properly run locally)

Happy to discuss it !

@codecov
Copy link

codecov bot commented Dec 12, 2025

Codecov Report

❌ Patch coverage is 86.55462% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.99%. Comparing base (36942b0) to head (c321694).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
client/pyroclient/client.py 45.00% 11 Missing ⚠️
src/app/api/api_v1/endpoints/poses.py 89.47% 4 Missing ⚠️
src/app/api/api_v1/endpoints/cameras.py 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #531      +/-   ##
==========================================
+ Coverage   83.91%   83.99%   +0.08%     
==========================================
  Files          39       42       +3     
  Lines        1212     1318     +106     
==========================================
+ Hits         1017     1107      +90     
- Misses        195      211      +16     
Flag Coverage Δ
backend 84.01% <94.94%> (+0.78%) ⬆️
client 83.67% <45.00%> (-10.00%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@fe51 fe51 changed the title [WIP] Introduces poses table and related endpoints udpates Introduces poses table and related endpoints udpates Dec 12, 2025
@fe51
Copy link
Member Author

fe51 commented Dec 12, 2025

@MateoLostanlen @frgfm happy to get your feedbacks !

@fe51 fe51 changed the title Introduces poses table and related endpoints udpates Introduces Poses table and related endpoints udpates Dec 15, 2025
Copy link
Member

@MateoLostanlen MateoLostanlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fe51, thnaks for the PR

few remarks

out need from .crud_pose import * insrc/app/crud/init.py and from .poses import * in src/app/schemas/init.py

@MateoLostanlen
Copy link
Member

We miss a route to get all poses associated to a camera

@fe51
Copy link
Member Author

fe51 commented Dec 19, 2025

We miss a route to get all poses associated to a camera

--> Camera endpoints have been specifically updated to return information about camera poses.
You can see it in the screenshot below or in endpoint camera

image

Copy link
Member

@MateoLostanlen MateoLostanlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi please can you update create_detection incleint too ? we need to pass a pose id now

@fe51 fe51 requested a review from MateoLostanlen December 19, 2025 17:26
@fe51
Copy link
Member Author

fe51 commented Dec 19, 2025

@MateoLostanlen

Hi please can you update create_detection incleint too ? we need to pass a pose id now

Nice catch, took in account your comment and had introduced pose_id as param.
We are close to merge :)

@MateoLostanlen
Copy link
Member

MateoLostanlen commented Dec 19, 2025

@MateoLostanlen

Hi please can you update create_detection incleint too ? we need to pass a pose id now

Nice catch, took in account your comment and had introduced pose_id as param. We are close to merge :)

just the corresponding test missing and we are good to go !

Copy link
Member

@MateoLostanlen MateoLostanlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good for me, thanks @fe51

@fe51 fe51 merged commit ac3e175 into main Dec 20, 2025
23 of 24 checks passed
@fe51 fe51 deleted the poses branch December 20, 2025 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants