Add Profile User model and add test examples for quality control - Iss42 & 31 #44
Conversation
|
Just as a suggestion, it's possible that this could have been split into two separate pull requests, one to create the user profile and one to do the trustable user part. The reason for this is to make it easier to think about just one new feature instead of multiple (possibly unrelated) changes in the same pull request. In this case the changes are quite related, so I'm not too worried, but for other changes you might want to consider two pull requests. |
| @@ -413,6 +414,7 @@ class Vote(models.Model): | |||
| annotation = models.ForeignKey(Annotation, related_name='votes') | |||
| visited_sound = models.NullBooleanField(null=True, blank=True, default=None) | |||
| # 'visited_sound' is to store whether the user needed to open the sound in Freesound to perform this vote | |||
| is_trustable = models.NullBooleanField(null=True, blank=True) # store if the user was trustable when he voted | |||
There was a problem hiding this comment.
what does null mean?
- can be trusted
- can't be trusted
- schrodinger's user
If you're using a boolean, it's much better to have only True/False values
There was a problem hiding this comment.
This is for quality control. is_trustable indicate if the user was trustable when he performed the validation task. Null is used because this quality control was not implemented during the Tagathon.
For now we consider that at this time, the user were trustable, but it could change in the future, so I keep this as null for the old votes.
There was a problem hiding this comment.
There's an issue here that you're wanting to store 3 values, and you're using null/True/False to do it.
If you really need to store three values (trustable, untrustable, unknown), it would be much better to use an Enum instead.
However, I'd consider if this is actually required. Is there a problem with marking all currently existing votes as non-trusted? You could mark them all is_trustable=False, then at a later stage as you mark users as trustable, go back and mark all of their historical votes as trusted too.
There was a problem hiding this comment.
I can consider using an Enum.
But for me this null/True/False makes a lot of sense. We did not have the information when the annotations where voted. It is what null means.
My experience in these things is limited, I trust you if you say that it is not right.
If we assume that they are all False, we "loose" all the 40k annotations (we consider them as not trustable, so we don't use them for the ground truth generation).
For a vote, we store the state of user.profile.is_trustable at the moment he performed the task. If one day he succeed in passing test cases, we don't want to change the is_trustable propriety of the votes that were performed when he did not pass the test cases.
datasets/models.py
Outdated
| class Profile(models.Model): | ||
| user = models.OneToOneField(User, on_delete=models.CASCADE) | ||
| is_trustable = models.BooleanField(default=False) | ||
| countdown_trustable = models.IntegerField(default=0) |
There was a problem hiding this comment.
What does this mean? It's difficult for me to understand it just by looking at the name. Either consider a different name, or add a comment here which explains what the field does.
datasets/views.py
Outdated
| @@ -189,19 +202,45 @@ def save_contribute_validate_annotations_category(request): | |||
| comment_form = CategoryCommentForm(request.POST) | |||
| formset = PresentNotPresentUnsureFormSet(request.POST) | |||
| if formset.is_valid() and comment_form.is_valid(): | |||
| test_annotations_id = [] # initiate it | |||
There was a problem hiding this comment.
this kind of comment is not needed
a Profile model has been implemented. User model has a One-To-One Link to Profile model
This way we can hold extra info about users (ie. is_trustable & countdown_trustable)
a is_trustable field has been added to Vote in order to store if the user was trustable when he submitted his votes
test examples are added to vote forms every 5 pages in order to check user reliability
NB: For now only positive test examples are used. We plan to add also negative examples.