Skip to content

feat: refactor validators and fields to support pydantic v2 #967

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

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

Conversation

xiaobo8204
Copy link
Contributor

Description

This PR aims to update Pydantic to the latest v2.10.6, and switch to use the new model_validator/Field/field_validator in the new Pydantic version.

Related Issue(s)

Checklist

  • [Y] I've read the CONTRIBUTING guidelines.
  • [Y] I've updated the documentation if applicable.
  • [Y] I've added tests if applicable.
  • [Y] @mentions of the person or team responsible for reviewing proposed changes.

@xiaobo8204 xiaobo8204 changed the title Upgrade Pydantic to v2.10.6: Migrated Validators, Field Enhancements #929 Upgrade Pydantic to v2.10.6: Migrated Validators, Field Enhancements Feb 3, 2025
@Pouyanpi Pouyanpi self-requested a review February 3, 2025 10:09
@Pouyanpi
Copy link
Collaborator

Pouyanpi commented Feb 3, 2025

@xiaobo8204 thanks for your contribution to NeMo Guardrails 👍🏻 would you please sign all your commits following the contributing guidelines. You should see a Verified tag for all your commits.

@Pouyanpi Pouyanpi added the enhancement New feature or request label Feb 3, 2025
@@ -65,7 +65,7 @@ langchain-community = ">=0.0.16,<0.4.0"
lark = ">=1.1.7"
nest-asyncio = ">=1.5.6,"
prompt-toolkit = ">=3.0"
pydantic = ">=1.10"
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should relax it a bit, probably >= 2.0

And I still need to see the reasons we were still supporting Pydantic 1.0, I'll get back to this later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the extra log.text and hidden file, changed the version to >= 2.0, and signed the commit, thanks...

Copy link
Collaborator

Choose a reason for hiding this comment

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

and <3.0

@Pouyanpi Pouyanpi changed the title Upgrade Pydantic to v2.10.6: Migrated Validators, Field Enhancements feat: refactor validators and fields to support pydantic v2 Feb 3, 2025
@xiaobo8204 xiaobo8204 force-pushed the feature/xiaobo_dev_features branch 2 times, most recently from 8581787 to bb87907 Compare February 3, 2025 20:44
@Pouyanpi Pouyanpi added this to the v0.13.0 milestone Feb 26, 2025
@Pouyanpi Pouyanpi self-assigned this Mar 3, 2025
@Pouyanpi Pouyanpi self-requested a review April 2, 2025 10:54
@Pouyanpi Pouyanpi added refactoring and removed enhancement New feature or request labels Apr 2, 2025
xiaobo and others added 2 commits April 11, 2025 13:10
- Refactored code to use `@model_validator` instead of `@root_validator`
- Updated `Field` usage to align with Pydantic v2 standards
- Modified validation logic to comply with the new API
- Updated `poetry.lock` and dependencies in `pyproject.toml`
@Pouyanpi Pouyanpi force-pushed the feature/xiaobo_dev_features branch from 6c0bf2c to 9af0695 Compare April 11, 2025 11:11
@Pouyanpi
Copy link
Collaborator

@xiaobo8204 Apologies for the delay in reviewing your PR.

The issue is that we currently don’t have proper tests for the code your PR affects. I’ll open a separate PR to add the necessary tests. Once that’s in place, we can rebase your branch and move forward from there. Thanks for your patience!

@@ -61,12 +61,12 @@ class TRTLLM(BaseLLM):
client: Any
streaming: Optional[bool] = True

Copy link
Collaborator

Choose a reason for hiding this comment

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

should be

    @classmethod
    def validate_environment(cls, self) -> "TRTLLM":
        """Validate that python package exists in environment."""
        try:
            # instantiate and attach the client
            self.client = TritonClient(self.server_url)

        except ImportError as err:
            raise ImportError(
                "Could not import triton client python package. "
                "Please install it with `pip install tritonclient[all]`."
            ) from err
        return self

@@ -147,11 +147,11 @@ class EvalConfig(BaseModel):
description="The prompts that should be used for the various LLM tasks.",
)

@root_validator(pre=False, skip_on_failure=True)
def validate_policy_ids(cls, values: Any):
@model_validator(mode="after")
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be

    @model_validator(mode="after")
    def validate_policy_ids(cls, self) -> "EvalConfig":
        """Validates the policy ids used in the interactions."""
        policy_ids = {policy.id for policy in self.policies}
        for interaction_set in self.interactions:
            for expected_output in interaction_set.expected_output:
                if expected_output.policy not in policy_ids:
                    raise ValueError(
                        f"Invalid policy id {expected_output.policy} used in interaction set."
                    )
            for policy_id in (
                interaction_set.include_policies + interaction_set.exclude_policies
            ):
                if policy_id not in policy_ids:
                    raise ValueError(
                        f"Invalid policy id {policy_id} used in interaction set."
                    )
        return self

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: Update Pydantic to v2.10.5 and remove Deprecated validators
2 participants