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

Improve docstrings + minor code refactor #545

Merged
merged 14 commits into from
Oct 4, 2024
Merged

Conversation

lhy-hoyin
Copy link
Contributor

@lhy-hoyin lhy-hoyin commented Oct 2, 2024

This PR includes:

  • Adding, formatting and improving of docstring comments
  • Updating absolute imports to relative imports (similar to other files)

Important

Improved docstrings, switched to relative imports, and used a constant for EMBEDDING_SIZE for better code clarity and consistency.

  • Docstrings:
    • Improved and formatted docstrings across multiple files for clarity and consistency.
    • Added detailed attribute descriptions in exception classes like AgentNotFoundError and UserNotFoundError.
  • Imports:
    • Updated absolute imports to relative imports in files like truncation.py, worker.py, and messages.py for consistency.

This description was created by Ellipsis for c27aebf. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to c27aebf in 36 seconds

More details
  • Looked at 1144 lines of code in 49 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. agents-api/agents_api/activities/truncation.py:6
  • Draft comment:
    Ensure consistency in import statements across the codebase. Some files have been updated to use relative imports, but others might still use absolute imports. Consider updating all relevant files for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR includes changes to update absolute imports to relative imports. However, there are some inconsistencies in the import statements across different files. For example, in agents-api/agents_api/activities/truncation.py, the import statement is changed to a relative import, but in other files, similar changes might be missing. It's important to ensure consistency across the codebase.
2. agents-api/agents_api/clients/worker/worker.py:3
  • Draft comment:
    Ensure consistency in import statements across the codebase. Some files have been updated to use relative imports, but others might still use absolute imports. Consider updating all relevant files for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR includes changes to update absolute imports to relative imports. However, there are some inconsistencies in the import statements across different files. For example, in agents-api/agents_api/clients/worker/worker.py, the import statement is changed to a relative import, but in other files, similar changes might be missing. It's important to ensure consistency across the codebase.
3. agents-api/agents_api/common/utils/messages.py:4
  • Draft comment:
    Ensure consistency in import statements across the codebase. Some files have been updated to use relative imports, but others might still use absolute imports. Consider updating all relevant files for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR includes changes to update absolute imports to relative imports. However, there are some inconsistencies in the import statements across different files. For example, in agents-api/agents_api/common/utils/messages.py, the import statement is changed to a relative import, but in other files, similar changes might be missing. It's important to ensure consistency across the codebase.
4. agents-api/tests/fixtures.py:40
  • Draft comment:
    Ensure consistency in import statements across the codebase. Some files have been updated to use relative imports, but others might still use absolute imports. Consider updating all relevant files for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR includes changes to update absolute imports to relative imports. However, there are some inconsistencies in the import statements across different files. For example, in agents-api/tests/fixtures.py, the import statement is changed to a relative import, but in other files, similar changes might be missing. It's important to ensure consistency across the codebase.
5. agents-api/tests/test_activities.py:10
  • Draft comment:
    Ensure consistency in import statements across the codebase. Some files have been updated to use relative imports, but others might still use absolute imports. Consider updating all relevant files for consistency.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR includes changes to update absolute imports to relative imports. However, there are some inconsistencies in the import statements across different files. For example, in agents-api/tests/test_activities.py, the import statement is changed to a relative import, but in other files, similar changes might be missing. It's important to ensure consistency across the codebase.

Workflow ID: wflow_QpsNuGpjGKs88P6c


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@creatorrr
Copy link
Contributor

This change needs a fix @lhy-hoyin . See the lint-and-type-check github action. It's throwing an error:

  File "<frozen importlib._bootstrap_external>", line 995, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/home/runner/work/julep/julep/agents-api/tests/test_activities.py", line 10, in <module>
    from tests.fixtures import cozo_client, test_developer_id, test_doc
  File "/home/runner/work/julep/julep/agents-api/tests/fixtures.py", line 39, in <module>
    from tests.utils import patch_embed_acompletion as patch_embed_acompletion_ctx
  File "/home/runner/work/julep/julep/agents-api/tests/utils.py", line 12, in <module>
    from tests.fixtures import EMBEDDING_SIZE
ImportError: cannot import name 'EMBEDDING_SIZE' from partially initialized module 'tests.fixtures' (most likely due to a circular import) (/home/runner/work/julep/julep/agents-api/tests/fixtures.py)

@lhy-hoyin
Copy link
Contributor Author

Hi @creatorrr , I have implemented a fix for the issue. Please assist to trigger GitHub Actions checks.

@creatorrr creatorrr merged commit fe8a8c9 into julep-ai:dev Oct 4, 2024
3 of 6 checks passed
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