Skip to content

Conversation

@vowelparrot
Copy link
Contributor

@vowelparrot vowelparrot commented Jun 22, 2023

  • Add protocol for evaluate_strings
  • Move the criteria evaluator out so it's not restricted to being applied on traced runs

@vercel
Copy link

vercel bot commented Jun 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jun 26, 2023 9:10pm

@vowelparrot vowelparrot force-pushed the vwp/string_eval_protocol branch from 78e2b33 to b1b9b16 Compare June 22, 2023 23:58
Copy link
Contributor

@nfcampos nfcampos 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

_SUPPORTED_CRITERIA.update(d)


class CriteriaEvalChain(LLMChain):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't we make this subclass StringEvaluator?

Copy link
Contributor Author

@vowelparrot vowelparrot Jun 26, 2023

Choose a reason for hiding this comment

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

Because it's a protocol and the metaclass differs from the chain. We could also make it just StringEvaluatorChain, which would probably be more conventional in langchain. I'll go ahead and do that

_SUPPORTED_CRITERIA.update(d)


def get_criteria_evaluator(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: instead of having this method here, can we just have an to_run_evaluator method on StringEvaluator that could convert any arbitrary StringEvaluator into a RunEvaluator (which expects a Run)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmmm yeah we could

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think I'd do in a subsequent PR

@vowelparrot vowelparrot force-pushed the vwp/string_eval_protocol branch 5 times, most recently from 79f8d13 to bdd6116 Compare June 26, 2023 21:08
@vowelparrot vowelparrot force-pushed the vwp/string_eval_protocol branch from bdd6116 to fd82f46 Compare June 26, 2023 21:10
@vowelparrot vowelparrot merged commit c460b04 into master Jun 26, 2023
@vowelparrot vowelparrot deleted the vwp/string_eval_protocol branch June 26, 2023 21:16
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.

4 participants