-
Notifications
You must be signed in to change notification settings - Fork 26
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
Implement rank column in caprieval
#213
Conversation
also some code linting
For example:
|
sort_ascending = true | ||
# which field should be used to create the ranking | ||
rankby = 'score' | ||
rank_ascending = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if both sort_ascending
and rank_ascending
are true and sortby
is set to e.g. i-rmsd while rankby
is set by score?
Shouldn't we have only sort_ascending
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if both
sort_ascending
andrank_ascending
are true andsortby
is set to e.g. i-rmsd whilerankby
is set by score?
This scenario is the example I posted above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But aren't sort_ascending
and rank_ascending
redundant??? I don't see the need for the rank_ascending
parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one defined the direction in which the table will be sorted, the other the direction of the value that we will rank the models;
for sorting: irmsd we'd sort asceding (the lower the best), fnat descending (the higher the best)
for ranking: haddock-score ascending (the lower the best), some other module in which the higher the best = descending
But up to you, I can remove it as well. Rule # 1 !
Ok - still a bit confusing though - let’s approve this one then
|
Currently the ranking is independent from the sorting, it can be ranked on one parameter and sorted on another, and in different directions. Is it better to make it so that the table will be ranked and sorted to only one parameter? |
Currently the ranking is independent from the sorting, it can be ranked on one parameter and sorted on the order in different directions.
Is it better to make it so that the table will be ranked and sorted to only one parameter?
No - Sorting and rankings are simply different things.
|
This PR implements a new ranking column in
caprieval
and closes #212.It adds two new parameters.
With the changes on this PR its possible to use one column to do the ranking and another for the sorting, let me know if this is not the desired behavior of the feature.