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

Pydantic foundations #153

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Conversation

JGoldstone
Copy link
Contributor

This PR reimplements camdkit on Pydantic foundations.

There are no more manually implemented validate(), to_json(), from_json() or even make_json_schema() methods.

The fundamental aggregating construct is the Pydantic BaseModel on which CompatibleBaseModel is built.

Changes in client code have been avoided wherever possible. There are a few things:

  • One can't construct invalid objects, even temporarily; and so the unit test pattern that first constructs an invalid object and then calls validate() on it can't be implemented.
  • It is no longer possible to construct objects the names of which are snake_case with purely positional calls. One of the most common cases of this style of construction was TimecodeFormat. In the Pydantic implementation, callers will need to replace

TimeCodeFormat(30)

with

TimecodeFormat(frame_rate=30)

No attempt has been made to Pydanticize any of opentrackio_lib.py, open.trackio_parser.py, opentrackio_receiver.py or opentrackio_sender.py. Personally I think in an ideal world we would have a Pydanticized parser and also a C++ implementation -- probably C++14 to make Steve's RHEL8 platforms happy -- and would test round-tripping starting at either end. But that's not touched on by this PR.

I am not sure why GitHub is telling me that this can't be automatically merged, but if someone wants to explain to me what I would need to do to remedy that I'd certainly be open to the idea. Right now I just want to get eyes on this.

JGoldstone and others added 23 commits January 23, 2025 15:34
* Change master to leader.

* Updated master to leader.

* Complete rewrite of library and parser.

* Example sender and receiver.

* Changed master to leader.

* Updated documentation with new header format.

* Removed section about RTP (for now).

* Updated based on feedback from Steve R.

* Removed residual reference to parentId.

---------

Co-authored-by: Marcus Bengtsson <[email protected]>
…ere we were passing in a list instead of a tuple.
@JGoldstone JGoldstone requested a review from jamesmosys January 31, 2025 06:44
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