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

Fix CSV ordering, run pre-commit #1764

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Fix CSV ordering, run pre-commit #1764

merged 2 commits into from
Jan 7, 2025

Conversation

NolanTrem
Copy link
Collaborator

@NolanTrem NolanTrem commented Jan 7, 2025

Important

Fix CSV column ordering in multiple database handlers and update logging in retrieval service.

  • CSV Export:
    • Fix column ordering in CSV exports by using a dictionary to map row data to specified columns in collections.py, conversations.py, documents.py, graphs.py, and users.py.
  • Logging:
    • Replace print with logger.warning in num_tokens_from_messages() in retrieval_service.py.

This description was created by Ellipsis for c535fce. 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 6000b1d in 1 minute and 21 seconds

More details
  • Looked at 297 lines of code in 8 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. py/core/database/collections.py:585
  • Draft comment:
    Ensure that the columns list is validated against row_dict keys to prevent KeyError if an invalid column name is passed.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_EI13u3KVOBjsften


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

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! Incremental review on c535fce in 25 seconds

More details
  • Looked at 24 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. py/core/database/users.py:935
  • Draft comment:
    The PR description mentions replacing print with logger.warning in retrieval_service.py, but this change is not present in the diff. Ensure that this change is included or update the PR description accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR description mentions replacing print with logger.warning in retrieval_service.py, but this file is not included in the diff. This might be an oversight or a separate commit.
2. py/core/database/users.py:1003
  • Draft comment:
    Ensure that similar changes in column ordering are applied consistently across all CSV export functions in other files (collections.py, conversations.py, documents.py, graphs.py) as mentioned in the PR description.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change in column ordering is correctly implemented, but it's important to ensure that all related CSV export functions across different files are updated similarly.

Workflow ID: wflow_UVukVNSW8Wi7jbob


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

@NolanTrem NolanTrem merged commit 2f63262 into main Jan 7, 2025
13 of 14 checks passed
@NolanTrem NolanTrem deleted the Nolan/CSVOrdering branch January 7, 2025 20:56
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.

1 participant