-
-
Notifications
You must be signed in to change notification settings - Fork 730
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
Update mode.py to warn in Mode.FUNCTIONS
access vs. in __new__
#678
Conversation
Sweep: PR Reviewinstructor/mode.pyThe No issues found with the reviewed changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to 90edc70 in 2 minutes and 8 seconds
More details
- Looked at
33
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_1n2HvCQH5zZpIZX4
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
This is a cross repository pull request, but Ellipsis isn't installed in boydgreenfield/instructor. In order to have Ellipsis address comments in this PR, you'll need to install Ellipsis in that repository. |
6cf9f85
to
b8168e5
Compare
Currently, any import of `instructor` raises a `DeprecationWarning`. This is noisy and also makes it impossible to have test suites raise an error on `DeprecationWarning`. Example code: ```python3 import warnings warnings.simplefilter("default") import instructor # raises `DeprecationWarning: FUNCTIONS is deprecated and will be removed in future versions enum_member = enum_class._new_member_(enum_class, *args)` ```
There was a problem hiding this 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 9ef6f1a in 52 seconds
More details
- Looked at
43
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. instructor/mode.py:6
- Draft comment:
The implementation of the deprecation warning in__getattribute__
is correct and should effectively warn users whenFUNCTIONS
is accessed. However, ensure that all other enum members are accessible without any unintended side effects or errors by thoroughly testing access to all other members of theMode
enum. - Reason this comment was not posted:
Confidence changes required:33%
The PR moves the deprecation warning for theFUNCTIONS
enum member from the__new__
method to the__getattribute__
method of a custom metaclass_WarnOnFunctionsAccessEnumMeta
. This change is intended to reduce the noise of deprecation warnings at import time, only triggering them when theFUNCTIONS
member is actually accessed. This is a good approach as it allows for more controlled handling of deprecation warnings, especially in test environments where warnings might be treated as errors or need special handling.
Workflow ID: wflow_KxL4gpsmY7MmwEvo
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Match changes as of instructor-ai#678
Currently, any import of
instructor
raises aDeprecationWarning
. This is noisy and also makes it impossible to have test suites raise an error onDeprecationWarning
. Example code:Summary:
This PR moves the deprecation warning for
FUNCTIONS
inMode
class to only trigger when accessed, reducing import-time noise.Key points:
DeprecationWarning
forFUNCTIONS
from__new__
to__getattribute__
in aMode
metaclass.DeprecationWarning
in test suites.Generated with ❤️ by ellipsis.dev
Summary:
This PR moves the deprecation warning for
FUNCTIONS
in theMode
class to only trigger when accessed, reducing import-time noise.Key points:
DeprecationWarning
forFUNCTIONS
from__new__
to__getattribute__
inMode
class using_WarnOnFunctionsAccessEnumMeta
metaclass.DeprecationWarning
in test suites.Generated with ❤️ by ellipsis.dev