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

Migrate to UV #1280

Merged
merged 23 commits into from
Dec 26, 2024
Merged

Migrate to UV #1280

merged 23 commits into from
Dec 26, 2024

Conversation

ivanleomk
Copy link
Collaborator

@ivanleomk ivanleomk commented Dec 25, 2024

We should be able to speed up CI significantly with this


Important

Migrates CI from poetry to uv for faster execution and suppresses type checking errors in instructor/client.py.

  • CI Setup:
    • Migrates CI from poetry to uv in pyright.yml, python-publish.yml, ruff.yml, test.yml, and test_docs.yml.
    • Removes poetry.toml.
  • Configuration:
    • Updates pyproject.toml to use uv configuration, replacing [tool.poetry] with [project] and [project.optional-dependencies].
    • Adjusts dependency and script sections to align with uv requirements.
  • Code Changes:
    • Adds # type: ignore[override] comments in instructor/client.py for create, create_partial, create_iterable, and create_with_completion methods to suppress type checking errors.
  • Misc:
    • Updates download count in README.md.

This description was created by Ellipsis for 6188bb1. 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 9439f2b in 18 seconds

More details
  • Looked at 301 lines of code in 3 files
  • Skipped 2 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/ruff.yml:31
  • Draft comment:
    Ensure that uv is correctly set up and configured before running uv sync --all-extras. Consider adding a step to verify the installation or configuration of uv.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR description mentions migrating to UV for faster CI. The changes in the workflow file reflect this, but there is a missing step to ensure the environment is correctly set up for UV.
2. pyproject.toml:16
  • Draft comment:
    Since the library code has changed with the introduction of uv, ensure that the documentation is updated accordingly. Also, verify that tests are updated to reflect these changes.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_ulg5D1VOOHrAJ11H


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

@github-actions github-actions bot added dependencies Pull requests that update a dependency file size:M This PR changes 30-99 lines, ignoring generated files. labels Dec 25, 2024
Copy link

cloudflare-workers-and-pages bot commented Dec 25, 2024

Deploying instructor-py with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6188bb1
Status: ✅  Deploy successful!
Preview URL: https://ab875eea.instructor-py.pages.dev
Branch Preview URL: https://migrate-to-uv.instructor-py.pages.dev

View logs

@ivanleomk
Copy link
Collaborator Author

Following this specific guide : https://x.com/tiangolo/status/1839686030007361803

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 8c5c14d in 33 seconds

More details
  • Looked at 121 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. .github/workflows/test.yml:29
  • Draft comment:
    Replace poetry with uv for running tests to align with the migration from poetry to uv.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. .github/workflows/test_docs.yml:36
  • Draft comment:
    Replace poetry with uv for running tests to align with the migration from poetry to uv.
  • Reason this comment was not posted:
    Marked as duplicate.
3. .github/workflows/python-publish.yml:36
  • Draft comment:
    Replace poetry with uv for getting the release version to align with the migration from poetry to uv.
  • Reason this comment was not posted:
    Comment was on unchanged code.
4. .github/workflows/test.yml:29
  • Draft comment:
    Replace poetry with uv for running tests to maintain consistency with the new setup.
  • Reason this comment was not posted:
    Marked as duplicate.
5. .github/workflows/test_docs.yml:36
  • Draft comment:
    Replace poetry with uv for running tests to maintain consistency with the new setup.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_kcH73pNzowKX2Leb


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 fb5ae36 in 9 seconds

More details
  • Looked at 33 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yml:29
  • Draft comment:
    Consider defining a reusable command or step for uvx run to avoid repetition and potential errors in future updates. This applies to lines 29, 36, 43, 44, and 45.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The uvx run command is used multiple times in the workflow file. It would be more efficient to define a reusable command or step to avoid repetition and potential errors in future updates.
2. .github/workflows/test.yml:30
  • Draft comment:
    Environment variables for API keys are repeated in multiple steps. Consider setting them once at the job level to adhere to the DRY principle.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The code has repetitive environment variable settings for API keys in multiple steps. This violates the DRY principle.

Workflow ID: wflow_FaYBMUNjPCATQN5X


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 17ee012 in 53 seconds

More details
  • Looked at 10 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/pyright.yml:33
  • Draft comment:
    Consider specifying a Pyright version to ensure consistency and avoid potential breaking changes from future updates.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The removal of the specific Pyright version could lead to inconsistencies if the latest version has breaking changes.
2. .github/workflows/pyright.yml:33
  • Draft comment:
    Consider specifying a version for jakebailey/pyright-action to ensure consistent behavior across runs.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The removal of the version specification for the pyright-action might lead to unexpected behavior if the action updates in a way that is not compatible with the current setup.

Workflow ID: wflow_FqF2zqwr62bRUFlu


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 d25ce3a in 32 seconds

More details
  • Looked at 69 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. pyproject.toml:25
  • Draft comment:
    The [tool.pyright] section was removed and then re-added without changes. Consider retaining it in its original position to avoid confusion.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The removal and re-addition of the [tool.pyright] section in pyproject.toml seems unnecessary and could lead to confusion. It should be retained in its original position if no changes are made.
2. pyproject.toml:88
  • Draft comment:
    The change in the pyright version specifier is a library code change. Ensure that the documentation and tests are updated accordingly.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The rules explicitly state "Do NOT comment on dependency changes, library versions that you don't recognize, or anything else related to dependencies." This is clearly a dependency version change in the dev dependencies section. The comment violates this rule directly.
    Maybe pyright is a special case since it's a type checker and could affect the codebase's type checking behavior?
    No, the rules are clear about dependency changes, and there's no exception listed for development tools. We should trust that the author has tested the dependency change appropriately.
    The comment should be deleted as it violates the explicit rule about not commenting on dependency changes.

Workflow ID: wflow_fhMZUdlyRZlVwd5F


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 ee87158 in 9 seconds

More details
  • Looked at 11 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/pyright.yml:34
  • Draft comment:
    Consider specifying the working directory for uv run pyright to ensure it runs in the correct context. Use --cwd ${{ env.WORKING_DIRECTORY }} if needed.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The uv run pyright command should specify the working directory to ensure it runs in the correct context.

Workflow ID: wflow_p4ziOjdc5zqUdw7l


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 bf9c1d2 in 12 seconds

More details
  • Looked at 40 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. instructor/client.py:376
  • Draft comment:
    Using # type: ignore[override] suggests a mismatch in method signatures between AsyncInstructor and Instructor. Ensure this is intentional and won't cause runtime issues.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of # type: ignore[override] indicates that the method signatures in the subclass AsyncInstructor do not match the base class Instructor. This could lead to potential issues if the method signatures are expected to be the same. It's important to ensure that the method signatures are intentionally different and that this won't cause any runtime issues.
2. instructor/client.py:376
  • Draft comment:
    Add a comment explaining why # type: ignore[override] is used for create, create_partial, create_iterable, and create_with_completion methods in AsyncInstructor.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The create, create_partial, create_iterable, and create_with_completion methods in the AsyncInstructor class have a # type: ignore[override] comment, which suggests that the method signatures might not match the base class. This needs to be documented or explained in the code.

Workflow ID: wflow_UOfGUexrQJDbbLgn


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 8377d14 in 11 seconds

More details
  • Looked at 33 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yml:29
  • Draft comment:
    Ensure that uv is the correct command replacement for uvx. If uvx was used for a specific reason, verify that uv provides the same functionality.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The uvx command is replaced with uv, which is correct as per the PR description. However, the uvx command is not used anywhere else in the file, so no further changes are needed.
2. .github/workflows/test.yml:29
  • Draft comment:
    The change from uvx to uv should be mentioned in the PR description for clarity.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from uvx to uv in the workflow file is consistent, but the PR description should mention this change for clarity.

Workflow ID: wflow_IWYE6rX5HuZfem2f


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 2e7cf73 in 13 seconds

More details
  • Looked at 20 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. pyproject.toml:94
  • Draft comment:
    pytest-examples is defined twice in the dev and docs groups with overlapping version constraints. Consider consolidating to avoid redundancy.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The pytest-examples dependency is defined twice in the dev and docs groups with overlapping version constraints. This redundancy should be removed to avoid confusion.
2. pyproject.toml:94
  • Draft comment:
    Duplicate entry for pytest-examples in dev and docs sections. Consider removing one to adhere to DRY principle.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The pyproject.toml file has duplicate entries for pytest-examples in both dev and docs sections. This violates the DRY principle.

Workflow ID: wflow_gPL8AssZn4lB7Ny7


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 c90f968 in 16 seconds

More details
  • Looked at 52 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. uv.lock:1763
  • Draft comment:
    Duplicate entry for pytest-examples in docs section. Consider removing it to avoid redundancy.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The uv.lock file has duplicate entries for pytest-examples in both the dev and docs sections. This redundancy should be removed to avoid potential conflicts or confusion.
2. uv.lock:1663
  • Draft comment:
    Ensure that documentation and tests are updated to reflect the change in the pytest-examples dependency version.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The change in the uv.lock file indicates an update to the pytest-examples dependency. This is a library code change, so we need to ensure that the documentation and tests are updated accordingly.

Workflow ID: wflow_vRWVPGyDfS6D7Ww4


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 3f9901c in 14 seconds

More details
  • Looked at 45 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yml:17
  • Draft comment:
    Consider updating to actions/checkout@v3 for consistency and to use the latest version.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of actions/checkout@v2 is outdated and should be updated to actions/checkout@v3 for consistency and to benefit from the latest features and fixes.
2. .github/workflows/pyright.yml:21
  • Draft comment:
    Ensure that documentation and tests are updated to reflect the change from poetry to uv. This is crucial for maintaining consistency and correctness.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_zfsQ5iCre7GHOS1b


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 9f2ba9e in 28 seconds

More details
  • Looked at 29 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/python-publish.yml:24
  • Draft comment:
    The order of setting up Python and installing uv is incorrect. uv should be installed after setting up Python.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The order of setting up Python and installing uv is incorrect. uv should be installed after setting up Python.
2. .github/workflows/python-publish.yml:31
  • Draft comment:
    The line using poetry to get the release version should be updated to use uv instead, as the rest of the workflow has migrated to uv.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_G4K924JffUn0HKnq


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 33e968a in 11 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/test.yml:35
  • Draft comment:
    Consider including the Gemini tests in the coverage report for completeness. Currently, the coverage report excludes them.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The condition for running the Gemini tests is correct, but the coverage report should also include the Gemini tests for completeness.
2. .github/workflows/test.yml:35
  • Draft comment:
    The condition if: matrix.python-version == '3.11' is repeated for running Gemini tests and generating coverage report. Consider refactoring to avoid repetition and adhere to the DRY principle.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The condition for running Gemini tests is repeated, which violates the DRY principle. It should be refactored to avoid repetition.

Workflow ID: wflow_M9BLjz3f7Ry0AnWO


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

@ivanleomk
Copy link
Collaborator Author

Just a quick note on how UV works here

uv add --dev pytest-examples

This helps to add a package as a optional dependency

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 ac8f7e8 in 16 seconds

More details
  • Looked at 14 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. pyproject.toml:25
  • Draft comment:
    The [tool.uv] section is not standard for pyproject.toml. Verify if this is necessary or correctly placed.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The [tool.uv] section is not standard for pyproject.toml. It might be a custom configuration, but it should be verified if it's necessary or correctly placed.
2. pyproject.toml:22
  • Draft comment:
    The test-docs optional dependency group is duplicated in the [dependency-groups] section. Please remove the duplicate to adhere to the DRY principle. This issue is also present for other groups like anthropic, litellm, google-generativeai, and vertexai.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The pyproject.toml file has duplicate entries for optional dependencies and dependency groups, which violates the DRY principle.

Workflow ID: wflow_TfdBQlnqSHSsNG0X


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 8679309 in 11 seconds

More details
  • Looked at 39 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. README.md:3
  • Draft comment:
    Ensure the download count is accurate and up-to-date.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The README update reflects a change in the number of downloads, which is a factual update and does not require any code changes. However, it's important to ensure that the number is accurate and up-to-date.
2. README.md:523
  • Draft comment:
    Ensure that the mkdocs.yml file is updated to include the new instructions for using uv as mentioned in the README.md.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The README.md file has been updated with new instructions for using uv, but the mkdocs.yml file should also be updated to reflect these changes.

Workflow ID: wflow_Dj1ATmoiAyap7XR8


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

@ivanleomk ivanleomk requested a review from jxnl December 25, 2024 16:19
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 f484c9f in 13 seconds

More details
  • Looked at 27 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. uv.lock:1599
  • Draft comment:
    Changing source from virtual to editable may affect package installation behavior. Ensure this aligns with the intended use of the package.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from 'virtual' to 'editable' in the 'uv.lock' file might affect how the package is installed. This should be verified to ensure it aligns with the intended behavior.
2. uv.lock:1599
  • Draft comment:
    Library code change detected. Ensure that documentation and tests are updated accordingly.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_JbLfzwuK3Xx85XR6


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 14ca299 in 42 seconds

More details
  • Looked at 38 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. instructor/client_anthropic.py:12
  • Draft comment:
    The enable_prompt_caching parameter is removed from the function signature but still mentioned in the docstring. Please update the docstring to reflect the current function parameters.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The removal of enable_prompt_caching from the function signature in instructor/client_anthropic.py is not reflected in the docstring. This can lead to confusion for developers using this function.
2. instructor/client_anthropic.py:25
  • Draft comment:
    The enable_prompt_caching parameter is removed from the function signature but still mentioned in the docstring. Please update the docstring to reflect the current function parameters.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The removal of enable_prompt_caching from the function signature in instructor/client_anthropic.py is not reflected in the docstring. This can lead to confusion for developers using this function.
3. instructor/utils.py:170
  • Draft comment:
    Initializing cache_creation_input_tokens and cache_read_input_tokens to 0 if they are not set is a good practice to prevent NoneType errors during arithmetic operations. This is correctly implemented here and in lines 173-174.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The code in instructor/utils.py initializes cache_creation_input_tokens and cache_read_input_tokens to 0 if they are not already set. This is a good practice to avoid NoneType errors during arithmetic operations.
4. instructor/utils.py:170
  • Draft comment:
    The initialization of cache_creation_input_tokens and cache_read_input_tokens to 0 is repeated. Consider refactoring to avoid repetition.
            for attr in ['cache_creation_input_tokens', 'cache_read_input_tokens']:
                if not getattr(total_usage, attr):
                    setattr(total_usage, attr, 0)
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The repetition is real but minor - just two lines. The initialization is part of a larger block handling Anthropic-specific usage tracking. A refactor might make the code slightly cleaner but could also make it less readable by separating related logic. The current form is very explicit and clear about what's happening.
    The comment points out a real DRY violation, but is the suggested refactor worth the added complexity? The current code is very readable and maintainable as is.
    While technically correct, this refactor would be a micro-optimization that could make the code less clear for minimal benefit. The current repetition serves readability.
    The comment should be deleted. While technically correct about the DRY violation, the suggested refactor would likely not improve the code enough to justify the change.

Workflow ID: wflow_4WXJJF9zzrUVwjla


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 980b0f2 in 21 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. uv.lock:1597
  • Draft comment:
    Ensure the version update in uv.lock is consistent with pyproject.toml.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The version update in uv.lock should be consistent with the rest of the project files. This change should be reflected in the pyproject.toml as well.
2. uv.lock:1597
  • Draft comment:
    If the version update in uv.lock reflects changes in the library, ensure that corresponding documentation and tests are updated as well.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The version update in uv.lock should be accompanied by corresponding updates in documentation and tests, if applicable.

Workflow ID: wflow_4nBrU7Q61BDdU9ka


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 6188bb1 in 14 seconds

More details
  • Looked at 14 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/python-publish.yml:34
  • Draft comment:
    Ensure uv publish handles versioning correctly, as the step to get the release version was removed.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The removal of the step to get the release version using poetry is correct since the workflow is now using uv. However, the uv publish command should be verified to ensure it handles versioning correctly.
2. .github/workflows/python-publish.yml:34
  • Draft comment:
    Consider adding a comment explaining why the 'Get release version' step was removed, if it was intentional. This will help future maintainers understand the change.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The removal of the 'Get release version' step might affect the workflow if the release version is needed elsewhere. However, since the PR description doesn't mention any issues with this, it seems intentional.

Workflow ID: wflow_iLHZjyw8Qfe2v1T7


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

@ivanleomk ivanleomk merged commit 415de0a into main Dec 26, 2024
14 of 15 checks passed
@ivanleomk ivanleomk deleted the migrate-to-uv branch December 26, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants