Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feature/benchmark_evaluator #68

Merged
merged 9 commits into from
May 18, 2024
Merged

Feature/benchmark_evaluator #68

merged 9 commits into from
May 18, 2024

Conversation

XuhuiZhou
Copy link
Member

Closes #

📑 Description

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed
  • Branch name follows type/descript (e.g. feature/add-llm-agents)
  • Ready for code review

ℹ Additional Information

@ProKil
Copy link
Member

ProKil commented May 16, 2024

It seems that the pre-commit breaks your lines, so the type: ignore is not on the line you meant to ignore.

@XuhuiZhou
Copy link
Member Author

Any chance you can quickly fix this?

@XuhuiZhou XuhuiZhou requested a review from ProKil May 17, 2024 21:11
@ProKil
Copy link
Member

ProKil commented May 17, 2024

Hi @XuhuiZhou Can you briefly explain what are the changes made to serialization in the PR description.

@XuhuiZhou
Copy link
Member Author

Basically just the rewards, previously I excluded the overall rewards but now I am adding them back

ProKil
ProKil requested changes May 18, 2024
View reviewed changes
Copy link
Member

@ProKil ProKil left a comment

Choose a reason for hiding this comment

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

As we want to make our code more usable, we should aim for easy-to-understand naming for variables, functions, and better docstring. The feature introduced by this PR is great, but please improve the naming to make code more self-explanatory.

sotopia/server.py Outdated Show resolved Hide resolved
tests/database/test_serialization.py Outdated Show resolved Hide resolved
sotopia/database/aggregate_annotations.py Outdated Show resolved Hide resolved
examples/benchmark_evaluator.py Outdated Show resolved Hide resolved
examples/benchmark_evaluator.py Outdated Show resolved Hide resolved
examples/benchmark_evaluator.py Outdated Show resolved Hide resolved
examples/benchmark_evaluator.py Outdated Show resolved Hide resolved
examples/benchmark_evaluator.py Outdated Show resolved Hide resolved
sotopia/database/aggregate_annotations.py Outdated Show resolved Hide resolved
@XuhuiZhou XuhuiZhou requested a review from ProKil May 18, 2024 14:48
@XuhuiZhou
Copy link
Member Author

As we want to make our code more usable, we should aim for easy-to-understand naming for variables, functions, and better docstring. The feature introduced by this PR is great, but please improve the naming to make code more self-explanatory.

Thanks for the comments!

@ProKil ProKil merged commit 1a49302 into main May 18, 2024
3 checks passed
@ProKil ProKil deleted the feature/benchmark_evaluator branch May 18, 2024 20:30
@ProKil ProKil mentioned this pull request May 19, 2024
Closed
15 tasks
@ProKil ProKil added this to the 0.1.0 Release milestone May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ProKil ProKil ProKil approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.1.0 Release
Development

Successfully merging this pull request may close these issues.
2 participants
@XuhuiZhou @ProKil