Skip to content

Conversation

@ihsaan-ullah
Copy link
Collaborator

@ihsaan-ullah ihsaan-ullah commented May 5, 2023

@ mention of reviewers

@Didayolo

A brief description of the purpose of the changes contained in this PR.

To solve the issue: Leaderboards require at least 1 column

Instead of leaderboard name, leaderboard label is now used to get right columns of the leaderboard.

Issues this PR resolves

#831
This was happening because of the difference in leaderboard name and label:
Screenshot 2023-05-05 at 4 13 29 PM

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

@Didayolo
Copy link
Member

Didayolo commented May 9, 2023

@ihsaan-ullah that is awesome that you work on this issue. I'll review it soon.

I think it also solve the first point of #375

@Didayolo Didayolo mentioned this pull request May 9, 2023
3 tasks
@Didayolo
Copy link
Member

Didayolo commented May 9, 2023

@ihsaan-ullah When I try to upload the V1 Iris bundle, the leaderboard column is still named "set1_prediction" instead of "Prediction score". Is that normal?

Capture d’écran 2023-05-09 à 11 53 27

@Didayolo
Copy link
Member

Didayolo commented May 9, 2023

Oh OK I get it this is related to the label of the leaderboard, not the labels of the columns

@Didayolo
Copy link
Member

Didayolo commented May 9, 2023

So, to sum up. That is a nice catch. When I tried to debug my bundle, I did not realized that my leaderboard was named "score", and not "results" like in the Iris example. It is a good fix because it make the V1 bundle upload more robust.

Another related issue that is still to be solved, is that the v1 unpacker does not read the label of columns, and uses keys instead (see first point of #375).

@Didayolo Didayolo merged commit ed94425 into develop May 9, 2023
@Didayolo Didayolo deleted the leaderboard_columns_v1 branch May 9, 2023 10:08
@ihsaan-ullah
Copy link
Collaborator Author

I am going to check the remaining issues in #375

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