-
Notifications
You must be signed in to change notification settings - Fork 4
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
Edr/sql db #22
Edr/sql db #22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments with questions and changes.
I would also like to know how you see it being integrated with the pipeline.
cfd137b
to
f9afb38
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to be merged with the main because it has changes related to the PR. For example, I added generate_frames_decord that would need frame_ids as output as well.
There appears to be a problem with how SQLAlchemy's type annotations work. Column[int] is incompatible with int, unless you install sqlalchemy-stubs, which breaks the type annotations for reference() fields. But you can manually cast things to int (id_type), so we do that.
rename SqlAlchemy models to ...Entity
30f0d2a
to
af68a85
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed a few issues.
Also, I think I figured out the problem with tests, please check if it solves the issue.
And run the ruff check, there are a few problems it reports.
8f92d47
to
94ba449
Compare
No description provided.