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.

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

store metric_names in the DB #492

Merged
merged 1 commit into from
Nov 5, 2022
Merged

store metric_names in the DB #492

merged 1 commit into from
Nov 5, 2022

Conversation

lyuyangh
Copy link
Member

@lyuyangh lyuyangh commented Nov 5, 2022

blocked by #491
part of #433
Stores metric_names in the DB because it is required when we use update_overall_statistics() to update the analysis. update_overall_statistics() does not depend on metadata anymore. It still depends on system_output_data though. I will remove that dependency in the next PR.

Compatibility

  • DB migration required
  • A new field is added to the returned System model. This is compatible with old versions of explainaboard_api_client so it shouldn't break anything. I tested explainaboard_api_client=0.2.11 (which is the oldest version we support) with this code and it worked OK.

Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

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.

2 participants