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

Avoid deprecated class property stacking #637

Merged
merged 4 commits into from
May 1, 2024

Conversation

max-muoto
Copy link
Contributor

@max-muoto max-muoto commented May 1, 2024

This PR removes stacking of property and classmethod, which is currently deprecated, and to be removed in Python 3.13. We simply create a custom classpropety decorator, which can replace this functionality as is.


🚀 This description was created by Ellipsis for commit 3e00eaa

Summary:

Introduces a custom classproperty decorator to replace deprecated stacking of property and classmethod, ensuring compatibility with future Python versions.

Key points:

  • Introduced classproperty to replace deprecated stacking of property and classmethod.
  • Modified OpenAISchema in /instructor/function_calls.py to use classproperty.
  • Defined classproperty in /instructor/utils.py with a detailed docstring.
  • Added tests for classproperty in /tests/test_utils.py.
  • Updated .ruff.toml and pyproject.toml for configuration changes.

Generated with ❤️ by ellipsis.dev

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 the entire pull request up to 3e00eaa
  • Looked at 158 lines of code in 5 files
  • Took 24 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 85%.
1. instructor/utils.py:193:
  • Assessed confidence : 0%
  • Comment:
    The implementation of classproperty is correct and follows the typical pattern for creating class-level properties in Python. Good job on this.
  • Reasoning:
    The PR introduces a new classproperty decorator to replace the deprecated stacking of classmethod and property. The implementation of classproperty in instructor/utils.py seems correct and follows the typical pattern for creating such a descriptor in Python. The PR also updates the usage of this new decorator in instructor/function_calls.py and adds tests for it in tests/test_utils.py. The PR description and changes align well, and the implementation seems to follow Python's best practices for descriptors.

Workflow ID: wflow_5ad09KUjlfwOxdzO


You can customize Ellipsis with review rules, user-specific overrides, quiet mode, and more. See docs.

Copy link
Collaborator

@jxnl jxnl 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!

@jxnl jxnl merged commit ab5b48d into instructor-ai:main May 1, 2024
8 of 12 checks passed
boydgreenfield pushed a commit to boydgreenfield/instructor that referenced this pull request May 17, 2024
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