Fix pylint errors and warnings in edited files

csm_web/scheduler/models.py

@@ -42,6 +42,7 @@ def day_to_number(day_of_week):
 
 
 def week_bounds(date):
+    """Compute the bounds of the week containing the date."""
     week_start = date - datetime.timedelta(days=date.weekday())
     week_end = week_start + datetime.timedelta(weeks=1)
     return week_start, week_end
@@ -51,6 +52,7 @@ class User(AbstractUser):
     priority_enrollment = models.DateTimeField(null=True, blank=True)
 
     def can_enroll_in_course(self, course, bypass_enrollment_time=False):
+        """Determine whether this user is allowed to enroll in the given course."""
         # check restricted first
         if course.is_restricted and not self.is_whitelisted_for(course):
             return False
@@ -61,17 +63,18 @@ def can_enroll_in_course(self, course, bypass_enrollment_time=False):
         )
         if bypass_enrollment_time:
             return not is_associated
+
+        if self.priority_enrollment:
+            now = timezone.now().astimezone(timezone.get_default_timezone())
+            is_valid_enrollment_time = (
+                self.priority_enrollment < now < course.enrollment_end
+            )
         else:
-            if self.priority_enrollment:
-                now = timezone.now().astimezone(timezone.get_default_timezone())
-                is_valid_enrollment_time = (
-                    self.priority_enrollment < now < course.enrollment_end
-                )
-            else:
-                is_valid_enrollment_time = course.is_open()
-            return is_valid_enrollment_time and not is_associated
+            is_valid_enrollment_time = course.is_open()
+        return is_valid_enrollment_time and not is_associated
 
     def is_whitelisted_for(self, course: "Course"):
+        """Determine whether this user is whitelisted for the given course."""
         return not course.is_restricted or self.whitelist.filter(pk=course.pk).exists()
 
     class Meta:
@@ -123,10 +126,12 @@ def __str__(self):
 
     @property
     def section(self):
+        """Retrieve the section associated with this attendance."""
         return self.student.section
 
     @property
     def week_start(self):
+        """Retrieve the date corresonding to the beginning of the week for this attendance."""
         return week_bounds(self.sectionOccurrence.date)[0]
 
     class Meta:
@@ -189,6 +194,7 @@ def clean(self):
             raise ValidationError("word of the day limit must be in days")
 
     def is_open(self):
+        """Determine whether the course is open for enrollment."""
         now = timezone.now().astimezone(timezone.get_default_timezone())
         return self.enrollment_start < now < self.enrollment_end
 
@@ -201,6 +207,7 @@ class Profile(ValidatingModel):
 
     @property
     def name(self):
+        """Retrieve the user's full name."""
         return self.user.get_full_name()
 
     def __str__(self):
@@ -232,10 +239,8 @@ class Student(Profile):
 
     def save(self, *args, **kwargs):
         super().save(*args, **kwargs)
-        """
-        Create an attendance for this week for the student if their section hasn't already been held this week
-        and no attendance for this week already exists.
-        """
+        # Create an attendance for this week for the student if their section
+        # hasn't already been held this week and no attendance for this week already exists.
         now = timezone.now().astimezone(timezone.get_default_timezone())
         week_start = week_bounds(now.date())[0]
         for spacetime in self.section.spacetimes.all():
@@ -255,14 +260,22 @@ def save(self, *args, **kwargs):
             ):
                 if settings.DJANGO_ENV != settings.DEVELOPMENT:
                     logger.info(
-                        "<SectionOccurrence> SO automatically created for student"
-                        f" {self.user.email} in course {course.name} for date"
-                        f" {now.date()}"
+                        (
+                            "<SectionOccurrence> SO automatically created for student"
+                            " %s in course %s for date %s"
+                        ),
+                        self.user.email,
+                        course.name,
+                        now.date(),
                     )
                     logger.info(
-                        "<Attendance> Attendance automatically created for student"
-                        f" {self.user.email} in course {course.name} for date"
-                        f" {now.date()}"
+                        (
+                            "<Attendance> Attendance automatically created for student"
+                            " %s in course %s for date %s"
+                        ),
+                        self.user.email,
+                        course.name,
+                        now.date(),
                     )
                 so_qs = SectionOccurrence.objects.filter(
                     section=self.section,
@@ -336,6 +349,7 @@ class Section(ValidatingModel):
 
     @functional.cached_property
     def current_student_count(self):
+        """Query the number of students currently enrolled in this section."""
         return self.students.filter(active=True).count()
 
     def delete(self, *args, **kwargs):
@@ -348,26 +362,28 @@ def delete(self, *args, **kwargs):
 
     def clean(self):
         super().clean()
-        """
-        Checking self.pk is checking if this is a creation (as opposed to an update)
-        We can't possibly have spacetimes at creation time (because it's a foreign-key field),
-        so we only validate this on updates
-        """
+        # Checking self.pk is checking if this is a creation (as opposed to an update)
+        # We can't possibly have spacetimes at creation time (because it's a foreign-key field),
+        # so we only validate this on updates
         if self.pk and not self.spacetimes.exists():
             raise ValidationError("Section must have at least one Spacetime")
 
     def __str__(self):
-        return "{course} section ({enrolled}/{cap}, {mentor}, {spacetimes})".format(
-            # pylint is unable to recognize the reverse accessor in the OneToOneOrNoneField
-            course=self.mentor.course.name,  # pylint: disable=no-member
-            mentor="(no mentor)" if not self.mentor else self.mentor.name,
-            enrolled=self.current_student_count,
-            cap=self.capacity,
-            spacetimes="|".join(map(str, self.spacetimes.all())),
+        # pylint is unable to recognize the reverse accessor in the OneToOneOrNoneField
+        course_name = self.mentor.course.name  # pylint: disable=no-member
+        enrolled_count = self.current_student_count
+        capacity = self.capacity
+        mentor_name = "(no mentor)" if not self.mentor else self.mentor.name
+        spacetimes = "|".join(map(str, self.spacetimes.all()))
+
+        return (
+            f"{course_name} section ({enrolled_count}/{capacity}, {mentor_name},"
+            f" {spacetimes})"
         )
 
 
 def worksheet_path(instance, filename):
+    """Compute the full worksheet path for a worksheet file."""
     # file will be uploaded to MEDIA_ROOT/<course_name>/<filename>
     course_name = str(instance.resource.course.name).replace(" ", "")
     return f"resources/{course_name}/{filename}"
@@ -404,7 +420,7 @@ class Worksheet(ValidatingModel):
 
 
 @receiver(models.signals.post_delete, sender=Worksheet)
-def auto_delete_file_on_delete(sender, instance, **kwargs):
+def auto_delete_file_on_delete(instance, **kwargs):
     """
     Deletes file from filesystem when corresponding
     `Worksheet` object is deleted.
@@ -416,13 +432,13 @@ def auto_delete_file_on_delete(sender, instance, **kwargs):
 
 
 @receiver(models.signals.pre_save, sender=Worksheet)
-def auto_delete_file_on_change(sender, instance, **kwargs):
+def auto_delete_file_on_change(instance, **kwargs):
     """
     Deletes old file from filesystem when corresponding
     `Worksheet` object is updated with a new file.
     """
     if not instance.pk:
-        return False
+        return
 
     db_obj = Worksheet.objects.get(pk=instance.pk)
     exists = True
@@ -465,6 +481,7 @@ class Spacetime(ValidatingModel):
 
     @property
     def override(self):
+        """Retrieve the override for this spacetime if it exists."""
         # pylint is unable to recognize the reverse accessor
         # pylint: disable=no-member
         return (
@@ -475,6 +492,7 @@ def override(self):
 
     @property
     def end_time(self):
+        """Retrieve the end time for this spacetime."""
         # Time does not support addition/subtraction,
         # so we have to create a datetime wrapper over start_time to add it to duration
         return (
@@ -490,6 +508,7 @@ def end_time(self):
         ).time()
 
     def day_number(self):
+        """Retrieve the day number corresponding to the day of week for this spacetime."""
         return day_to_number(self.day_of_week)
 
     def __str__(self):
@@ -524,6 +543,7 @@ def clean(self):
             )
 
     def is_expired(self):
+        """Determine whether this override for the spacetime is expired."""
         now = timezone.now().astimezone(timezone.get_default_timezone())
         return self.date < now.date()
 

csm_web/scheduler/views/spacetime.py

@@ -23,15 +23,23 @@ def get_queryset(self):
         ).distinct()
 
     def destroy(self, request, pk=None):
+        """
+        Deletes a spacetime, if possible.
+
+        Will not delete the spacetime if it will leave no spacetimes associated with a section.
+        """
         spacetime = get_object_or_error(self.get_queryset(), pk=pk)
         section = spacetime.section
         course = section.mentor.course
 
         is_coordinator = course.coordinator_set.filter(user=request.user).exists()
         if not is_coordinator:
             logger.error(
-                "<Spacetime Deletion:Failure> Could not delete spacetime, user"
-                f" {log_str(request.user)} does not have proper permissions"
+                (
+                    "<Spacetime Deletion:Failure> Could not delete spacetime, user %s"
+                    " does not have proper permissions"
+                ),
+                log_str(request.user),
             )
             raise PermissionDenied(
                 "You must be a coordinator to delete this spacetime!"
@@ -40,8 +48,8 @@ def destroy(self, request, pk=None):
         has_multiple_spacetimes = section.spacetimes.count() > 1
         if not has_multiple_spacetimes:
             logger.error(
-                f"<Spacetime Deletion:Failure> Could not delete spacetime, only one"
-                f" spacetime exists"
+                "<Spacetime Deletion:Failure> Could not delete spacetime, only one"
+                " spacetime exists"
             )
             return Response(
                 status=status.HTTP_400_BAD_REQUEST,
@@ -50,17 +58,19 @@ def destroy(self, request, pk=None):
 
         now = timezone.now().astimezone(timezone.get_default_timezone())
 
-        # FIX ME: Once issue #303 is resolved, this should delete the directly related section occurences
-        future_sectionOccurrences = section.sectionoccurrence_set.filter(
+        # FIX ME: Once issue #303 is resolved, this should delete
+        # the directly related section occurences
+        future_section_occurrences = section.sectionoccurrence_set.filter(
             date__gte=now.date(), date__week_day=spacetime.day_number()
         )
 
         with transaction.atomic():
             spacetime.delete()
-            future_sectionOccurrences.delete()
+            future_section_occurrences.delete()
 
         logger.info(
-            f"<Spacetime Deletion:Success> Deleted Spacetime {log_str(spacetime)}"
+            "<Spacetime Deletion:Success> Deleted Spacetime %s",
+            log_str(spacetime),
         )
 
         return Response(status=status.HTTP_200_OK)
@@ -86,17 +96,20 @@ def modify(self, request, pk=None):
         # validate and save
         spacetime.save()
         logger.info(
-            f"<Spacetime:Success> Modified Spacetime {log_str(spacetime)} (previously"
-            f" {old_spacetime_str})"
+            "<Spacetime:Success> Modified Spacetime %s (previously %s)",
+            log_str(spacetime),
+            old_spacetime_str,
         )
         return Response(status=status.HTTP_202_ACCEPTED)
 
     @action(detail=True, methods=["put", "delete"])
     def override(self, request, pk=None):
+        """Either update or delete a spacetime override."""
         spacetime = get_object_or_error(self.get_queryset(), pk=pk)
 
         if request.method == "PUT":
             if hasattr(spacetime, "_override"):  # update
+                # pylint: disable-next=protected-access
                 serializer = OverrideSerializer(spacetime._override, data=request.data)
                 status_code = status.HTTP_202_ACCEPTED
             else:  # create
@@ -107,39 +120,47 @@ def override(self, request, pk=None):
             if serializer.is_valid():
                 override = serializer.save()
                 logger.info(
-                    f"<Override:Success> Overrode Spacetime {log_str(spacetime)} with"
-                    f" Override {log_str(override)}"
+                    "<Override:Success> Overrode Spacetime %s with Override %s",
+                    log_str(spacetime),
+                    log_str(override),
                 )
                 return Response(status=status_code)
             logger.error(
-                f"<Override:Failure> Could not override Spacetime {log_str(spacetime)},"
-                f" errors: {serializer.errors}"
+                "<Override:Failure> Could not override Spacetime %s, errors: %s",
+                log_str(spacetime),
+                serializer.errors,
             )
             return Response(
                 serializer.errors, status=status.HTTP_422_UNPROCESSABLE_ENTITY
             )
 
-        elif request.method == "DELETE":
+        if request.method == "DELETE":
             mentor = spacetime.section.mentor
             course = mentor.course
 
             is_mentor = mentor.user == request.user
             is_coordinator = course.coordinator_set.filter(user=request.user).exists()
             if not (is_mentor or is_coordinator):
                 logger.error(
-                    "<Override Deletion:Failure> Could not delete override, user"
-                    f" {log_str(request.user)} does not have proper permissions"
+                    (
+                        "<Override Deletion:Failure> Could not delete override, user %s"
+                        " does not have proper permissions"
+                    ),
+                    log_str(request.user),
                 )
                 raise PermissionDenied(
                     "You must be a mentor or a coordinator to delete this spacetime"
                     " override!"
                 )
 
             if hasattr(spacetime, "_override"):
-                override = spacetime._override
+                override = spacetime._override  # pylint: disable=protected-access
                 override.delete()
 
             logger.info(
-                f"<Override Deletion:Success> Deleted override for {log_str(spacetime)}"
+                "<Override Deletion:Success> Deleted override for %s",
+                log_str(spacetime),
             )
             return Response(status=status.HTTP_200_OK)
+
+        return Response(status=status.HTTP_404_NOT_FOUND)