Skip to content

Conversation

@andreibratu
Copy link

No description provided.

@andreibratu andreibratu merged commit aad3b49 into master Feb 13, 2025
3 checks passed
Comment on lines +288 to +290
const evaluatorsWithCallable = evaluators.filter(
(e) => e.callable !== undefined,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

this could potentially break existing callers, right? What if someone is explicitly passing callable: null?

Copy link
Author

Choose a reason for hiding this comment

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

image Screenshot 2025-02-14 at 09 16 41

I think it's well handled, the problem was that we could not specify an online evaluator

Comment on lines +94 to +96
callable?: Function;
/**The type of arguments the Evaluator expects - only required for local Evaluators.*/
argsType?: EvaluatorArgumentsType;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect we should/could do some stronger typing here where we can statically check if a passed evaluator is either a remote evaluator (path/id) or local one.

Copy link
Author

Choose a reason for hiding this comment

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

true, evaluator could be a union where you either provide a path or {callable, argsType, returnType}

Copy link
Author

Choose a reason for hiding this comment

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

image

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.

3 participants