Skip to content

Conversation

@vietluong2110
Copy link

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?

Add tapi option for tap models api

What does this PR do?

Add tapir option for tap models api.
Propose a fatory design pattern for tap apis.

References

#54

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

#58

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.

Is this a breaking change?

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

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.

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

@vietluong2110
Copy link
Author

@sfmig I've followed the API design from #58 for integrating Tapir's API into Co-Tracker. Additionally, I propose adopting the factory design pattern to simplify codebase management, making it easier to add more models in the future.

@sfmig
Copy link
Member

sfmig commented Apr 1, 2025

Hi @vietluong2110, thanks for having a go at this!

Would you mind having a look at the CI checks? There seem to be issues with linkcheck when building the docs, and also pre-commit and linting problems. Have a look at the contributing guidelines, they should help you get this sorted (esp the contributing code section).

Additionally, I noticed you added tapnet as a submodule. For maintainability, we prefer to add other packages as dependencies. Would you mind making this change too?

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