Refactor do_label

grader/applications.py

@@ -429,13 +429,37 @@ def get_labels(self, fullname):
         key = fullname.lower()
         return self[f'labels.{key}'] or []
 
+    def add_label(self, fullname, label):
+        old_labels = set(self.get_labels(fullname))
+        if label in old_labels:
+            return False
+
+        old_labels.add(label)
+        self.set_labels(fullname, sorted(old_labels))
+        return True
+
+    def remove_label(self, fullname, label):
+        old_labels = set(self.get_labels(fullname))
+        if label not in old_labels:
+            return False
+
+        old_labels.remove(label)
+        self.set_labels(fullname, sorted(old_labels))
+        return True
+
     def set_labels(self, fullname, labels):
         key = fullname.lower()
 
-        if not labels:
+        old_labels = set(self.get_labels(fullname))
+        new_labels = set(labels)
+        mod = old_labels == new_labels
+
+        if not new_labels:
             self.data['labels'].pop(key)
         else:
-            self[f'labels.{key}'] = labels
+            # write new labels
+            self[f'labels.{key}'] = sorted(labels)
+        return mod
 
     @property
     def formula(self):
@@ -494,7 +518,14 @@ def __getitem__(self, key):
             case int(key):
                 return self.people[key]
             case str(key):
-                return self.filter(fullname=f'^{key.lower()}$')[0]
+                filtered = self.filter(fullname=f'^{key.lower()}$')
+                match len(filtered):
+                    case 1:
+                        return filtered[0]
+                    case 0:
+                        raise IndexError(f"No person with name {key}")
+                    case _:
+                        raise IndexError(f"Multiple people with name {key}")
             case _:
                 raise TypeError
 

grader/grader.py

@@ -1095,6 +1095,15 @@ def do_equiv(self, args):
         self.config['equivs'][variant] = saved
         self.ranking_done = False
 
+
+
+    label_options = (
+        cmd_completer.PagedArgumentParser('label')
+            .add_argument('arg',
+                          nargs='+',
+                          help="Full name or index of the person to add a label to")
+    )
+
     def do_label(self, args):
         """Mark persons with string labels
 
@@ -1104,26 +1113,33 @@ def do_label(self, args):
         label                      # display all labels
         label LABEL                # display people thus labelled
         """
+
+        opts = self.label_options.parse_args(args.split())
+
         applications = self.applications
-        if args == '':
+        if not opts.arg:
             for applicant in applications:
                 if applicant.labels:
                     print('{} = {}'.format(applicant.fullname.lower(),
                                            applicant.labels))
             return
 
-        if '=' in args:
-            fullname, *labels = [item.strip() for item in args.split('=')
-                                 if item != '']
+        if '=' in opts.arg:
+            idx = opts.arg.index('=')
+            fullname = " ".join(opts.arg[:idx])
+            labels = opts.arg[idx+1:]
+
             if labels:
-                applications.add_labels(fullname, labels)
+                for label in labels:
+                    applications.ini.add_label(fullname, label)
             else:
-                applications.clear_labels(fullname)
+                applications.ini.set_labels(fullname, [])
         else:
-            display_by_label = any(label in set(args.split())
-                                   for label in applications.get_all_labels())
+            fullname = " ".join(opts.arg)
+            display_by_label = any(label in opts.arg
+                                   for label in applications.all_labels())
             if display_by_label:
-                for label in args.split():
+                for label in opts.arg:
                     count = 0
                     printf('== {} ==', label)
                     for applicant in applications:
@@ -1132,12 +1148,12 @@ def do_label(self, args):
                             count += 1
                     printf('== {} labelled ==', count)
             else:
-                applicant = applications.find_applicant_by_fullname(args)
+                applicant = applications[fullname]
                 labels = applicant.labels
                 if labels:
-                    printf('{} = {}', args, labels)
+                    printf('{} = {}', fullname, labels)
                 else:
-                    printf('{} has no labels', args)
+                    printf('{} has no labels', fullname)
 
     do_label.completions = _complete_name
 
@@ -1177,7 +1193,7 @@ def do_write(self, args):
         _write_file('list_custom_answer.csv',
                     applications.filter(label=('CUSTOM-ANSWER')))
         # get all INVITESL? labels
-        all_labels = self.applications.get_all_labels()
+        all_labels = self.applications.all_labels()
         invitesl = [label for label in all_labels
                     if label.startswith('INVITESL')]
         for i, sl_label in enumerate(invitesl):

grader/person.py

@@ -190,31 +190,23 @@ def add_label(self, label):
         if self._ini is None:
             raise ValueError
 
-        labels = self.labels
-        if label in self.labels:
-            return False
-
-        labels = sorted(labels + [label])
-        self._ini.set_labels(self.fullname, labels)
+        mod = self._ini.add_label(self.fullname, label)
 
-        # The internal state has been modified, increase generation number
-        self._generation += 1
-        return True
+        if mod:
+            # The internal state has been modified, increase generation number
+            self._generation += 1
+        return mod
 
     def remove_label(self, label):
         if self._ini is None:
             raise ValueError
 
-        labels = self.labels
-        if label not in self.labels:
-            return False
-
-        labels.remove(label)
-        self._ini.set_labels(self.fullname, labels)
+        mod = self._ini.remove_label(self.fullname, label)
 
-        # The internal state has been modified, increase generation number
-        self._generation += 1
-        return True
+        if mod:
+            # The internal state has been modified, increase generation number
+            self._generation += 1
+        return mod
 
     @property
     def fullname(self) -> str: