-
Notifications
You must be signed in to change notification settings - Fork 6
Add ratings for study documents. #397
base: master
Are you sure you want to change the base?
Conversation
| 'allow_summary': 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.
Nit: this empty line is inconsequenctial with the rest of the schema.
cburchert
left a comment
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.
Good code. Nice tests.
I would mainly suggest to return a small positive number for unrated things. We get that the rating is always a number and never None and also we rank unrated stuff over downvoted-only stuff.f
|
|
||
|
|
||
| def init_rating(items): | ||
| """On creating of a study-document, set the rating to None.""" |
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.
On creation
| ], | ||
| 'studydocumentratings': [ | ||
| # Rate such that: | ||
| # 0: No rating, 1: Low rating, 2: High rating |
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.
This is a slightly weird ordering... But that is just what the math works out to. We could in theory give unrated things a slightly positive epsilon rating to put them above things only voted negative.
|
Thanks for the review, and you raise a valid point with the ordering. However, any default value also seems a bit weird to me, in that case there is no clear way to distinguish a 'slightly' bad studydoc from an unrated one. Aside from those concerns, where would you put this threshold? I.e. at which rating would you place 'bad' documents under unrated ones? |
|
Not sure what you mean by 'slightly bad' studydoc. You would need a huge number of votes to get to an epsilon lower bound, right? Basically I would just return something like 1e-6 (large enough that it doesn't disappear in floating accuracy) from compute_rating() if there are no votes at all. As all cases where there are 0 upvotes, but >0 downvotes result in a value of 0 already, the ranking would always be:
There is of course some uncertainty for things that have 0 upvotes and 1 downvote and thus drop below unrated things, but I would guess this is still better than having the None value. Another more complex suggestion would be to order things with the same lower confidence bound by their upper confidence bound. That would have a similar effect, but would also rank the stuff with only downvotes by ascending downvotes. |
| (1 + (z**2)/total)) | ||
|
|
||
| # Ensure that the bound is not below 0 | ||
| return max(bound, 0) |
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.
Can bound mathematically even be lower than 0?
|
I have thought about this issue again. I think it might be best to just return more to the client, i.e. return both the upvotes, downvotes as well as the rating. This way, the client can still sort by rating, but show upvotes and downvotes, which might be more informative to the common user than the rating. |
Add a rating for study documents, via a new endpoint
studydocumentratingsto rate, and a fieldratingto display the result.We do not simply return the average vote, which can be quite far off, since we don't have many votes in general.
Instead, we return the lower bound of a statistical confidence interval.
On the one hand, it is useful, but actually I thought it's a neat approach, and since it is not much more complicated than an average rating I went for it^^
(The actual formula is a bit more complex, but thats it)
More info :)
While I was updating the studydocs, I also cleaned up the old hooks. It still included a hook that does not exist in Eve anymore (since a few versions ago, there are no separate hooks for creating one or multiple items).
Closes #89