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

[Feature] Comply codebase with mypy and add test coverage #153

Merged
merged 16 commits into from
May 3, 2024

Conversation

the-praxs
Copy link
Member

📥 Pull Request

📘 Description
This is still a WIP and the list will be updated regularly

  • Fixed mypy errors and docstring for Client class in client.py

🔄 Related Issue (if applicable)
#14

🎯 Goal
To make the codebase better by adding type checks and adding coverage in tests

🔍 Additional Context
None at the moment

🧪 Testing
Describe the tests you performed to validate your changes.

Thank you for your contribution to Agentops!

@the-praxs the-praxs marked this pull request as draft April 17, 2024 12:06
@the-praxs the-praxs marked this pull request as ready for review April 17, 2024 19:45
@the-praxs
Copy link
Member Author

2 errors that remain from mypy check -

  • X | Y syntax for unions requires Python 3.10 - Backwards compatibility will require changing the codebase to use the Union type instead of the | operator
  • Cannot find implementation or library stub for module named "<module>" - Occurs for the langchain, openai, and tenacity libraries. This is not shown when the --ignore-missing-imports arg is used with mypy. A permanent solution would be to generate stub files for these libraries.

@the-praxs
Copy link
Member Author

the-praxs commented Apr 17, 2024

@areibman can you please approve the workflow for this PR?

@areibman
Copy link
Contributor

@areibman can you please approve the workflow for this PR?

Thanks for doing this! Taking a look

agentops/client.py Outdated Show resolved Hide resolved
@the-praxs the-praxs requested a review from bboynton97 April 18, 2024 15:50
@bboynton97
Copy link
Contributor

Is this in a state ready to review? :)

@the-praxs
Copy link
Member Author

Is this in a state ready to review? :)

I will update some more over the weekend since I need to look at the new changes. Should be ready by Monday.

@bboynton97
Copy link
Contributor

@the-praxs just checking in on this! :)

@bboynton97 bboynton97 enabled auto-merge (squash) May 3, 2024 22:52
@bboynton97 bboynton97 merged commit 1c0e17f into AgentOps-AI:main May 3, 2024
1 of 2 checks passed
@the-praxs the-praxs deleted the feature/mypy branch July 27, 2024 06:16
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.

3 participants