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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove URLs in pydantic error messages #662

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ssonal
Copy link
Contributor

@ssonal ssonal commented May 12, 2024

Fixes #509

Pydantic includes error URLs in ValidationErrors which means extra tokens used in LLM calls.

Pydantic relies on an environment variable to control this setting and is on by default. https://github.com/pydantic/pydantic-core/blob/e1fc99dd3207157aad77defc20ab6873fd268b5b/python/pydantic_core/_pydantic_core.pyi#L818

This change sets the environment variable dynamically when the package is invoked.


馃殌 This description was created by Ellipsis for commit 5c2a359

Summary:

This PR sets an environment variable to remove URLs from Pydantic error messages and includes a test to verify the change.

Key points:

  • Set PYDANTIC_ERRORS_INCLUDE_URL to 0 in set_env_variable in /instructor/utils.py
  • Call set_env_variable in /instructor/__init__.py
  • Add test in /tests/test_function_calls.py to verify URLs are not in Pydantic error messages

Generated with 鉂わ笍 by ellipsis.dev

Fixes jxnl#509

Pydantic includes error URLs in ValidationErrors which means extra
tokens used in LLM calls.

Pydantic relies on an environment variable to control this setting and
is on by default. https://github.com/pydantic/pydantic-core/blob/e1fc99dd3207157aad77defc20ab6873fd268b5b/python/pydantic_core/_pydantic_core.pyi#L818

This change sets the environment variable dynamically when the package
is invoked.
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 5c2a359 in 32 seconds

More details
  • Looked at 70 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. instructor/__init__.py:26
  • Draft comment:
    Setting environment variables at runtime can lead to inconsistent behavior across different parts of the application, especially in a multi-threaded or asynchronous context. Consider setting this environment variable outside the application scope, for example through configuration management or directly in the environment where the application runs.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The PR aims to remove URLs from Pydantic error messages by setting an environment variable. The implementation sets the environment variable PYDANTIC_ERRORS_INCLUDE_URL to "0" in the set_env_variable function in utils.py. This function is called in __init__.py to ensure the variable is set when the package is used. The test test_pylance_url_config in test_function_calls.py checks if the URL is not included in the error message, which aligns with the PR's goal. However, setting environment variables at runtime can be problematic because it might not affect already imported modules or those imported in parallel at startup. This could lead to inconsistent behavior where some parts of the application still show URLs in error messages.

Workflow ID: wflow_iQ2aBd5qo3ROeUh0


You can customize Ellipsis with 馃憤 / 馃憥 feedback, review rules, user-specific overrides, quiet mode, and more.

@jxnl
Copy link
Owner

jxnl commented May 16, 2024

I would prefer this to be a piece of the documentation rather than something that we force users to use.

@ssonal
Copy link
Contributor Author

ssonal commented May 16, 2024

This does sound like a sane default to me, especially when retries come into the picture. Would be cumbersome to have to set this env variable each time. But maybe something like a config file makes more sense instead?

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.

Pydantic includes URLs with validation errors which aren't useful for the LLM
2 participants