Merge pull request #531 from ubc/master-all-possible-comparisons

Allow all valid comparisons

compair/algorithms/pair/adaptive/pair_generator.py

@@ -49,40 +49,19 @@ def generate_pair(self, scored_objects, comparison_pairs):
         if len(self.scored_objects) < 2:
             raise InsufficientObjectsForPairException
 
-        if self._has_compared_all():
-            raise UserComparedAllObjectsException
-
         comparison_pair = self._find_pair()
 
         if comparison_pair == None:
             raise
 
         return comparison_pair
 
-    def _has_compared_all(self):
-        """
-        Returns True if the user has already compared all objects, False otherwise.
-        """
-        compared_keys = set()
-        for comparison_pair in self.comparison_pairs:
-            compared_keys.add(comparison_pair.key1)
-            compared_keys.add(comparison_pair.key2)
-
-        all_keys = set([scored_object.key for scored_object in self.scored_objects])
-
-        # some comparison keys may have been soft deleted hence we need to use
-        # the set subtraction operation instead of comparing sizes
-        all_keys -= compared_keys
-
-        return not all_keys
-
-
     def _find_pair(self):
         """
         Returns an comparison pair by matching them up by score.
         - First key is selected by random within the lowest round possible
         - First key must have a valid opponent with the current user or else its skipped
-        - Second key canidates are filters by previous opponents to first key
+        - Second key candidates are filters by previous opponents to first key
         - Second key is selected by most similar score (randomly if tied) in the lowest round possible
         """
         score_object_1 = None
@@ -104,7 +83,6 @@ def _find_pair(self):
             if score_object_1 != None:
                 break
 
-        # Note this should be caught in get_pair
         if score_object_1 == None:
             raise UserComparedAllObjectsException
 

compair/algorithms/pair/random/pair_generator.py

@@ -44,40 +44,19 @@ def generate_pair(self, scored_objects, comparison_pairs):
         if len(self.scored_objects) < 2:
             raise InsufficientObjectsForPairException
 
-        if self._has_compared_all():
-            raise UserComparedAllObjectsException
-
         comparison_pair = self._find_pair()
 
         if comparison_pair == None:
             raise
 
         return comparison_pair
 
-    def _has_compared_all(self):
-        """
-        Returns True if the user has already compared all objects, False otherwise.
-        """
-        compared_keys = set()
-        for comparison_pair in self.comparison_pairs:
-            compared_keys.add(comparison_pair.key1)
-            compared_keys.add(comparison_pair.key2)
-
-        all_keys = set([scored_object.key for scored_object in self.scored_objects])
-
-        # some comparison keys may have been soft deleted hence we need to use
-        # the set subtraction operation instead of comparing sizes
-        all_keys -= compared_keys
-
-        return not all_keys
-
-
     def _find_pair(self):
         """
         Returns an comparison pair by matching them up randomly within a round.
         - First key is selected by random within the lowest round possible
         - First key must have a valid opponent with the current user or else its skipped
-        - Second key canidates are filters by previous opponents to first key
+        - Second key candidates are filters by previous opponents to first key
         - Second key is selected randomly in the lowest round possible
         """
         score_object_1 = None
@@ -99,7 +78,6 @@ def _find_pair(self):
             if score_object_1 != None:
                 break
 
-        # Note this should be caught in get_pair
         if score_object_1 == None:
             raise UserComparedAllObjectsException
 

compair/api/assignment.py

@@ -415,7 +415,8 @@ def get(self, course_uuid, assignment_uuid):
             .all()
 
         comparison_count = assignment.completed_comparison_count_for_user(current_user.id)
-        comparison_availble = Comparison.comparison_avialble_for_user(course.id, assignment.id, current_user.id)
+        other_student_answers = assignment.student_answer_count - answer_count
+        comparison_available = comparison_count < other_student_answers * (other_student_answers - 1) / 2
 
         status = {
             'answers': {
@@ -425,7 +426,7 @@ def get(self, course_uuid, assignment_uuid):
                 'draft_ids': [draft.uuid for draft in drafts]
             },
             'comparisons': {
-                'available': comparison_availble,
+                'available': comparison_available,
                 'count': comparison_count,
                 'left': max(0, assignment.total_comparisons_required - comparison_count)
             }
@@ -526,7 +527,8 @@ def get(self, course_uuid):
             )
             assignment_drafts = [draft for draft in drafts if draft.assignment_id == assignment.id]
             comparison_count = assignment.completed_comparison_count_for_user(current_user.id)
-            comparison_availble = Comparison.comparison_avialble_for_user(course.id, assignment.id, current_user.id)
+            other_student_answers = assignment.student_answer_count - answer_count
+            comparison_available = comparison_count < other_student_answers * (other_student_answers - 1) / 2
 
             statuses[assignment.uuid] = {
                 'answers': {
@@ -536,7 +538,7 @@ def get(self, course_uuid):
                     'draft_ids': [draft.uuid for draft in assignment_drafts]
                 },
                 'comparisons': {
-                    'available': comparison_availble,
+                    'available': comparison_available,
                     'count': comparison_count,
                     'left': max(0, assignment.total_comparisons_required - comparison_count)
                 }

compair/models/assignment.py

@@ -193,6 +193,21 @@ def __declare_last__(cls):
             group="counts"
         )
 
+        cls.student_answer_count = column_property(
+            select([func.count(Answer.id)]).
+            select_from(join(Answer, UserCourse, UserCourse.user_id == Answer.user_id)).
+            where(and_(
+                Answer.assignment_id == cls.id,
+                Answer.active == True,
+                Answer.draft == False,
+                Answer.practice == False,
+                UserCourse.course_id == cls.course_id,
+                UserCourse.course_role == CourseRole.student
+            )),
+            deferred=True,
+            group="counts"
+        )
+
         cls.top_answer_count = column_property(
             select([func.count(Answer.id)]).
             select_from(join(Answer, UserCourse, UserCourse.user_id == Answer.user_id)).

compair/models/comparison.py

@@ -76,40 +76,6 @@ def convert_to_comparison_pair(self):
             winning_key=self.winner_id
         )
 
-    @classmethod
-    def comparison_avialble_for_user(cls, course_id, assignment_id, user_id):
-        from . import UserCourse, CourseRole, Answer
-        # ineligible authors - eg. instructors, TAs, dropped student, user
-        ineligible_users = UserCourse.query. \
-            filter(and_(
-                UserCourse.course_id == course_id,
-                UserCourse.course_role != CourseRole.student
-            )). \
-            values(UserCourse.user_id)
-        ineligible_user_ids_base = [u[0] for u in ineligible_users]
-        ineligible_user_ids_base.append(user_id)
-
-        # ineligible authors (potentially) - eg. authors for answers that the user has seen
-        compared = Comparison.query \
-            .filter_by(
-                user_id=user_id,
-                assignment_id=assignment_id
-            ) \
-            .all()
-        compared_authors1 = [c.answer1.user_id for c in compared]
-        compared_authors2 = [c.answer2.user_id for c in compared]
-        ineligible_user_ids = ineligible_user_ids_base + compared_authors1 + compared_authors2
-
-        eligible_answers = Answer.query \
-            .filter(and_(
-                Answer.assignment_id == assignment_id,
-                Answer.user_id.notin_(ineligible_user_ids),
-                Answer.draft == False,
-                Answer.active == True
-            )) \
-            .count()
-        return eligible_answers / 2 >= 1  # min 1 pair required
-
     @classmethod
     def __declare_last__(cls):
         super(cls, cls).__declare_last__()

compair/tests/algorithms/test_pair_adaptive.py

@@ -69,12 +69,15 @@ def test_generate_pair(self, mock_shuffle):
                 rounds=3, wins=None, loses=None, opponents=None
             )
         ]
-        comparisons = [
-            ComparisonPair(1,2,None),
-            ComparisonPair(1,3,None),
-            ComparisonPair(1,4,None)
-        ]
+        comparisons = []
+        max_comparisons = 6 # n(n-1)/2 = 4*3/2
+        for i in range(0, max_comparisons):
+            results = self.pair_algorithm.generate_pair(scored_objects, comparisons)
+            comparisons.append(
+                ComparisonPair(results.key1, results.key2, results.key1)
+            )
 
+        # if trying to run one more time, should run into all objects compared error
         with self.assertRaises(UserComparedAllObjectsException):
             results = self.pair_algorithm.generate_pair(scored_objects, comparisons)
 

compair/tests/algorithms/test_pair_random.py

@@ -69,12 +69,15 @@ def test_generate_pair(self, mock_shuffle):
                 rounds=3, wins=None, loses=None, opponents=None
             )
         ]
-        comparisons = [
-            ComparisonPair(1,2,None),
-            ComparisonPair(1,3,None),
-            ComparisonPair(1,4,None)
-        ]
+        comparisons = []
+        max_comparisons = 6 # n(n-1)/2 = 4*3/2
+        for i in range(0, max_comparisons):
+            results = self.pair_algorithm.generate_pair(scored_objects, comparisons)
+            comparisons.append(
+                ComparisonPair(results.key1, results.key2, results.key1)
+            )
 
+        # if trying to run one more time, should run into all objects compared error
         with self.assertRaises(UserComparedAllObjectsException):
             results = self.pair_algorithm.generate_pair(scored_objects, comparisons)
 

compair/tests/api/test_assignment.py

@@ -370,8 +370,8 @@ def _submit_all_possible_comparisons_for_user(self, user_id):
         for answer in self.data.get_student_answers():
             if answer.assignment_id == self.assignment.id and answer.user_id != user_id:
                 num_eligible_answers += 1
-        # n - 1 possible pairs before all answers have been compared
-        num_possible_comparisons = num_eligible_answers - 1
+        # n(n-1)/2 possible pairs before all answers have been compared
+        num_possible_comparisons = int(num_eligible_answers * (num_eligible_answers - 1) / 2)
         for i in range(num_possible_comparisons):
             comparisons = Comparison.create_new_comparison_set(self.assignment.id, user_id, False)
             for comparison in comparisons:
@@ -462,8 +462,8 @@ def _submit_all_possible_comparisons_for_user(self, user_id):
         for answer in self.data.get_student_answers():
             if answer.assignment_id == self.assignment.id and answer.user_id != user_id:
                 num_eligible_answers += 1
-        # n - 1 possible pairs before all answers have been compared
-        num_possible_comparisons = num_eligible_answers - 1
+        # n(n-1)/2 possible pairs before all answers have been compared
+        num_possible_comparisons = int(num_eligible_answers * (num_eligible_answers - 1) / 2)
         for i in range(num_possible_comparisons):
             comparisons = Comparison.create_new_comparison_set(self.assignment.id, user_id, False)
             for comparison in comparisons:

compair/tests/api/test_comparisons.py

@@ -229,15 +229,19 @@ def test_get_and_submit_comparison(self, mocked_update_assignment_grades_run, mo
 
         users = [self.data.get_authorized_student(), self.data.get_authorized_instructor(), self.data.get_authorized_ta()]
         for user in users:
-            compared_answer_uuids = set()
+            max_comparisons = 0
+            other_student_answers = 0
             valid_answer_uuids = set()
             for answer in self.data.get_student_answers():
                 if answer.assignment.id == self.assignment.id and answer.user_id != user.id:
+                    other_student_answers += 1
                     valid_answer_uuids.add(answer.uuid)
+            max_comparisons = int(other_student_answers * (other_student_answers - 1) / 2)
 
             if user.id == self.data.get_authorized_student().id:
                 for comparison_example in self.data.comparisons_examples:
                     if comparison_example.assignment_id == self.assignment.id:
+                        max_comparisons += 1
                         valid_answer_uuids.add(comparison_example.answer1_uuid)
                         valid_answer_uuids.add(comparison_example.answer2_uuid)
 
@@ -254,7 +258,7 @@ def test_get_and_submit_comparison(self, mocked_update_assignment_grades_run, mo
                     db.session.commit()
 
                 current = 0
-                while len(valid_answer_uuids - compared_answer_uuids) > 0:
+                while current < max_comparisons:
                     current += 1
                     if user.id == self.data.get_authorized_student().id:
                         course_grade = CourseGrade.get_user_course_grade(self.course, user).grade
@@ -308,8 +312,6 @@ def test_get_and_submit_comparison(self, mocked_update_assignment_grades_run, mo
                         content_type='application/json')
                     self.assert200(rv)
                     actual_comparisons = rv.json['objects']
-                    compared_answer_uuids.add(actual_comparisons[0]['answer1_id'])
-                    compared_answer_uuids.add(actual_comparisons[0]['answer2_id'])
                     self._validate_comparison_submit(comparison_submit, actual_comparisons, expected_comparisons)
 
                     # grades should increase for every comparison
@@ -401,8 +403,8 @@ def _submit_all_possible_comparisons_for_user(self, user_id):
         for answer in self.data.get_student_answers():
             if answer.assignment_id == self.assignment.id and answer.user_id != user_id:
                 num_eligible_answers += 1
-        # n - 1 possible pairs before all answers have been compared
-        num_possible_comparisons = num_eligible_answers - 1
+        # n(n-1)/2 possible pairs before all answers have been compared
+        num_possible_comparisons = int(num_eligible_answers * (num_eligible_answers - 1) / 2)
         winner_ids = []
         loser_ids = []
         for i in range(num_possible_comparisons):
@@ -461,7 +463,7 @@ def test_score_calculation(self, mock_shuffle):
 
         # Check that ranking by score and by wins match, this only works for low number of
         # comparisons
-        sorted_expect_ranking = sorted(num_wins_by_id.items(), key=operator.itemgetter(0), reverse=True)
+        sorted_expect_ranking = sorted(num_wins_by_id.items(), key=operator.itemgetter(0))
         sorted_expect_ranking = sorted(sorted_expect_ranking, key=operator.itemgetter(1))
         expected_ranking_by_wins = [answer_id for (answer_id, wins) in sorted_expect_ranking]
 

compair/tests/test_compair.py

@@ -4,6 +4,8 @@
 import unittest
 import mock
 import uuid
+import sys
+import os
 
 from flask_testing import TestCase
 from os.path import dirname
@@ -64,6 +66,15 @@ def collect_from_response(resp_key, response):
         return wrapper
     return _decorator
 
+@contextmanager
+def suppress_stdout():
+    old_stdout = sys.stdout
+    sys.stdout = open(os.devnull, "w")
+    try:
+        yield
+    finally:
+        sys.stdout.close()
+        sys.stdout = old_stdout
 
 class ComPAIRTestCase(TestCase):
     def create_app(self):
@@ -72,7 +83,8 @@ def create_app(self):
 
     def setUp(self):
         db.create_all()
-        populate(default_data=True)
+        with suppress_stdout():
+            populate(default_data=True)
         # reset login settings in case tests fail
         self.app.config['APP_LOGIN_ENABLED'] = True
         self.app.config['CAS_LOGIN_ENABLED'] = True
@@ -197,7 +209,8 @@ def get_url(self, **values):
 class ComPAIRAPIDemoTestCase(ComPAIRAPITestCase):
     def setUp(self):
         db.create_all()
-        populate(default_data=True, sample_data=True)
+        with suppress_stdout():
+            populate(default_data=True, sample_data=True)
         self.app.config['DEMO_INSTALLATION'] = True