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.

Sign up for GitHub

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

Jump to bottom

Add tests for split_paragraph_chunks function #6

Closed
wants to merge 10 commits into from
Closed

Add tests for split_paragraph_chunks function #6

wants to merge 10 commits into from

Conversation

sweep-ai[bot]
Copy link
Contributor

@sweep-ai sweep-ai bot commented Oct 21, 2023

PR Feedback (click)

  • 👍 Sweep Did Well
  • 👎 Sweep Needs Improvement

Description

This PR adds tests for the split_paragraph_chunks function in the aicord/utils/string.py file. The function is responsible for splitting a large text into smaller chunks by paragraphs, with each chunk being less than a specified maximum length. The tests ensure that the function correctly splits the text into chunks of the appropriate size and that each chunk contains the correct text.

Summary of Changes

  • Created a new file string_test.py in the aicord/utils/ directory to house the tests for the split_paragraph_chunks function.
  • Imported the necessary modules for testing, including unittest and unittest.mock, as well as the split_paragraph_chunks function from aicord/utils/string.py.
  • Created a test class TestSplitParagraphChunks that inherits from unittest.TestCase.
  • Implemented multiple test methods within the TestSplitParagraphChunks class to cover various scenarios, such as text shorter than the maximum length, text exactly equal to the maximum length, text longer than the maximum length, and text with multiple paragraphs.
  • Each test method is annotated with @unittest.mock.patch as necessary to mock any dependencies.
  • Added docstrings to each test method to describe what is being tested.

Please review and merge this PR to ensure the correctness of the split_paragraph_chunks function and improve the overall test coverage of the repository.

Fixes #5.

🎉 Latest improvements to Sweep:

  • Sweep can now passively improve your repository! Check out Rules to learn more.

💡 To get Sweep to edit this pull request, you can:

  • Comment below, and Sweep can edit the entire PR
  • Comment on a file, Sweep will only modify the commented file
  • Edit the original issue to get Sweep to recreate the PR from scratch

@sweep-ai
Copy link
Contributor Author

sweep-ai bot commented Oct 21, 2023

Rollback Files For Sweep

  • Rollback changes to aicord/utils/string_test.py

@sweep-ai
Copy link
Contributor Author

sweep-ai bot commented Oct 21, 2023

Apply Sweep Rules to your PR?

  • Apply: There should not be large chunks of code that are just commented out. Docstrings and explanations in code are acceptable.
  • Apply: We should use loguru for error logging. If the log is inside an exception, use loguru.exception to add tracebacks. Use f-strings for string formatting in logger calls.
  • Apply: There should be no instances of import pdb; pdb.set_trace() in production code.
  • Apply: There should be no debug log or print statements in production code.
  • Apply: All functions should have parameters and output annotated with type hints. Use list, tuple and dict instead of typing.List, typing.Tuple and typing.dict.
  • Apply: Leftover TODOs in the code should be handled.
  • Apply: All new business logic should have corresponding unit tests in the same directory. For example, sweepai/api_test.py tests sweepai/api.py. Use unittest and unittest.mock as required.
  • Apply: Any clearly inefficient or repeated code should be optimized or refactored.
  • Apply: Remove any comments before code that are obvious. For example # this prints hello world; print('hello world').
  • Apply: Avoid duplicating code. Refactor and update duplicate code to improve maintainability.
  • Apply: Use consistent and descriptive commit messages that accurately reflect the changes made. Use gitmoji for commit messages.
  • Apply: Handle exceptions gracefully and provide appropriate error handling and logging.

@sweep-ai sweep-ai bot added the sweep label Oct 21, 2023
@sweep-ai sweep-ai bot mentioned this pull request Oct 21, 2023
Closed
8 tasks
leon0399
leon0399 requested changes Oct 21, 2023
View reviewed changes
aicord/utils/string_test.py Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Move this file to tests/utils/string_tests.py

Copy link
Contributor Author

@sweep-ai sweep-ai bot Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

I wasn't able to make changes. This could be due to an unclear request or a bug in my code.
As a reminder, comments on a file only modify that file. Comments on a PR (at the bottom of the "conversation" tab) can modify the entire PR. Please try again or contact us on Discord

Copy link
Member

Choose a reason for hiding this comment

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

This file should be stored in /tests/utils/string_tests.py, rather then /aicord/utils/string_test.py

Copy link
Contributor Author

@sweep-ai sweep-ai bot Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

I wasn't able to make changes. This could be due to an unclear request or a bug in my code.
As a reminder, comments on a file only modify that file. Comments on a PR (at the bottom of the "conversation" tab) can modify the entire PR. Please try again or contact us on Discord

aicord/utils/string_test.py Outdated
from aicord.utils.string import split_paragraph_chunks


class TestSplitParagraphChunks(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

Write a test, where multiple paragraphs are smaller than max length, so that 2 paragraphs fit into 1 chunk, but 3rrd one doesn't

Copy link
Contributor Author

@sweep-ai sweep-ai bot Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

leon0399

This comment was marked as outdated.

Sign in to view

leon0399
leon0399 reviewed Oct 21, 2023
View reviewed changes
tests/utils/string_test.py Outdated
"""
Test case where multiple paragraphs are smaller than max length, so that 2 paragraphs fit into 1 chunk, but 3rd one doesn't.
"""
text = "This is the first paragraph. " + "This is the second paragraph.\n\n" + "This is the third paragraph."
Copy link
Member

Choose a reason for hiding this comment

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

This test does not actually test splitting paragraphs, each paragraph is represented by \n\n, but it is only once here. Please, double check other functions also

Copy link
Contributor Author

@sweep-ai sweep-ai bot Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

leon0399
leon0399 requested changes Oct 21, 2023
View reviewed changes
Copy link
Member

@leon0399 leon0399 left a comment

Choose a reason for hiding this comment

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

Add GitHub action to run tests

@leon0399
Copy link
Member

@sweep-ai Please, add GitHub action to run tests

@sweep-ai
Copy link
Contributor Author

sweep-ai bot commented Oct 21, 2023 •
edited
Loading

🚀 Wrote Changes

Done.

leon0399
leon0399 requested changes Oct 21, 2023
View reviewed changes
.github/workflows/test.yml Outdated
- name: Set up Python 3.10
uses: actions/setup-python@v2
with:
python-version: 3.10
Copy link
Member

Choose a reason for hiding this comment

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

use exact version 3.10.13

Copy link
Contributor Author

@sweep-ai sweep-ai bot Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

Copy link
Member

Choose a reason for hiding this comment

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

  1. Use exact version 3.10.13
  2. Make it a string

Copy link
Contributor Author

@sweep-ai sweep-ai bot Oct 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

.github/workflows/test.yml Show resolved Hide resolved
@leon0399 leon0399 closed this Oct 21, 2023
@leon0399 leon0399 deleted the sweep/add-tests-split-paragraph-chunks branch October 21, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leon0399 leon0399 leon0399 requested changes

Assignees
No one assigned
Labels
sweep
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@leon0399