-
Notifications
You must be signed in to change notification settings - Fork 356
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
Fix validator initialization bugs #553
Conversation
…ARelevanceLLMEval, RemoveRedundantSentences, SaliencyCheck
@thekaranacharya good work on this. I went back and looked at the history of |
Got it. So basically if users define a |
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.
We should fast follow this PR with adding Pydantic 1.x and 2.x examples on the custom validators page: https://www.guardrailsai.com/docs/concepts/validators/#custom-validators
Right now it only shows how to use them in a RAIL spec.
Bug Description
As found in: #550, error was thrown when using
PydanticFieldValidator
due to a missing keyword argument in thesuper.__init__
call within the validator initialization method. This is a bug and all keyword argument(s) that a validator expects should be passed explicitly to the super call as passing in**kwargs
doesn't automatically include the remaining keyword arguments.Turns out, this is a pattern that has been erroneously implemented across the following 6 other validators alongwith the
PydanticFieldValidator
:BugFreeSQL
ExtractedSummarySentencesMatch
ExtractiveSummary
QARelevanceLLMEval
RemoveRedundantSentences
SaliencyCheck
Fix
All other validators pass in the keyword arguments explicitly in either of the following two ways:
super.__init__(on_fail, kwarg_1=kwarg_1, kwarg_2=kwarg_2,... **kwargs)
super.__init__(kwarg_1=kwarg_1, kwarg_2=kwarg_2,... on_fail=on_fail, **kwargs)
The
super.__init__
method's definition is as follows:guardrails/guardrails/validator_base.py
Line 196 in 8833c05
Hence, both of the ways are fine and the arguments get passed in correctly. This PR corrects these 7 validators by using the first way (either is fine)
A relevant similar PR was previously opened for
ProvenanceV0
: #288