Change the input type of the ReachGoalLLMEvaluator

[bugsz]

Following the discussion in https://github.com/sotopia-lab/sotopia/pull/123#discussion_r1659212670. I change the input type annotation from str to EnvResponseType (i.e. EnvResponseType = Union[EnvResponse, EnvResponsePlus, EnvResponseGoalOnly]). Also modify the naming a little bit.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 61.86%. Comparing base (4559cfa) to head (8000fc2).

@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
- Coverage   62.01%   61.86%   -0.16%     
==========================================
  Files          55       55              
  Lines        2733     2727       -6     
==========================================
- Hits         1695     1687       -8     
- Misses       1038     1040       +2     

Files	Coverage Δ
sotopia/cli/benchmark/benchmark.py	23.43% <ø> (ø)
sotopia/envs/evaluators.py	89.50% <100.00%> (-1.57%)	:arrow_down:
tests/envs/test_evaluators.py	100.00% <100.00%> (ø)
sotopia/server.py	16.02% <0.00%> (ø)

[ProKil]

Thanks @bugsz! This is quick and great.

We can use Generic + TypeVar to make this more extendable.

[bugsz]

Thanks for the commit @ProKil ! But I am a bit concerned about not having a default argument. Currently we are using SotopiaDimension everywhere except testing with single goal. Having a default would could be quite helpful

[ProKil]

You can add an alias with the fixed response_class_format and T_eval_dim?

[bugsz]

Actually I have tried adding a default, but seems it does not work Code: response_format_class: type[EvaluationForTwoAgents[T_eval_dim]] = EvaluationForTwoAgents[SotopiaDimensions], Issue: Incompatible default for argument "response_format_class" (default has type "type[EvaluationForTwoAgents[Any]]", argument has type "type[EvaluationForTwoAgents[T_eval_dim]]"

[ProKil]

No it won’t work. Generic class couldn’t get a default type, and you cannot get around by providing a default argument with the type annotation

On Fri, Jul 5, 2024 at 7:04 PM Zhe Su @.***> wrote:

Actually I have tried adding a default, but seems it does not work Code: response_format_class: type[EvaluationForTwoAgents[T_eval_dim]] = EvaluationForTwoAgents[SotopiaDimensions], Issue: Incompatible default for argument "response_format_class" (default has type "type[EvaluationForTwoAgents[Any]]", argument has type "type[EvaluationForTwoAgents[T_eval_dim]]"

— Reply to this email directly, view it on GitHub https://github.com/sotopia-lab/sotopia/pull/129#issuecomment-2211491479, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD436FQJ53ZQ462DT2X2QG3ZK4Q6RAVCNFSM6AAAAABKH2KDXGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJRGQ4TCNBXHE . You are receiving this because you were mentioned.Message ID: @.***>

[ProKil]

@bugsz If you want to add an alias like def NewClass(): return OldClass(arg=default_value) then feel free to do so.

[bugsz]

I see. I think maybe we just go ahead with the current impl?

[ProKil]

Sounds great!