Fixup docstrings & comments

- Typo fixes
- Better (but not perfect!) PEP 257 & PEP 8 compliance

Author: encukou

naucse/cli.py

@@ -5,8 +5,7 @@
 
 
 def cli(app, *, base_url=None, freezer=None):
-    """ Extends the elsa command line interface with a new command which prints all courses and runs
-        which are present with basic info about them.
+    """Return the elsa CLI extended with naucse-specific commands.
     """
     elsa_group = elsa.cli(app, base_url=base_url, freezer=freezer, invoke_cli=False)
 
@@ -18,14 +17,16 @@ def naucse():
     @click.option("--forks-only", default=False, is_flag=True,
                   help="Only list courses and runs from forks")
     def list_courses(forks_only):
-        """ List all courses and runs and info about them.
+        """List all courses and runs and info about them.
 
-        Mainly useful for courses from forks, shows where do they sourced from and if
-        they return even the most basic information and will therefore be included in
+        Mainly useful for courses from forks.
+        Shows where they are sourced from and if they return even the
+        most basic information and will therefore be included in
         list of courses/runs.
 
-        A practical benefit is that on Travis CI the docker images are pulled/built
-        in this command and the freezing won't timeout after the 10 minute limit if things are taking particularly long.
+        A practical benefit is that on Travis CI, the docker images are
+        pulled/built by this command, so freezing won't timeout after
+        the 10 minute limit if things are taking particularly long.
         """
         from naucse.views import model
 

naucse/freezer.py

@@ -6,26 +6,30 @@
 
 
 class AllLinksLogger(UrlForLogger):
-    """ AllLinksLogger primarily logs ``url_for`` calls, but yields urls from  ``absolute_urls_to_freeze`` as well.
+    """Logs ``url_for`` calls, but yields urls from ``absolute_urls_to_freeze`` as well.
     """
 
     def iter_calls(self):
-        """ Yields all logged urls and links parsed from content.
-            Unfortunately, ``yield from`` cannot be used as the queues are modified on the go.
+        """Yield all logged urls and links parsed from content.
         """
+        # Unfortunately, ``yield from`` cannot be used as the queues are
+        # modified on the go.
         while self.logged_calls or absolute_urls_to_freeze:
             if self.logged_calls:
                 yield self.logged_calls.popleft()
-                # prefer urls from :atrr:`logged_calls` - so, ideally, cache is populated from the base repository
+                # prefer urls from :atrr:`logged_calls` - so, ideally,
+                # cache is populated from the base repository
                 continue
             if absolute_urls_to_freeze:
                 yield absolute_urls_to_freeze.popleft()
 
 
 @contextlib.contextmanager
 def temporary_url_for_logger(app):
-    """ A context manager which temporary adds a new UrlForLogger to the app and yields it, so it can be used
-        to get logged calls.
+    """Context manager which temporary adds a new UrlForLogger to the app.
+
+    The logger is yielded as the context object, so it can be used
+    to get logged calls.
     """
     logger = UrlForLogger(app)
 
@@ -40,4 +44,6 @@ class NaucseFreezer(Freezer):
 
     def __init__(self, app):
         super().__init__(app)
-        self.url_for_logger = AllLinksLogger(app)  # override the default url_for_logger with our modified version
+
+        # override the default url_for_logger with our modified version
+        self.url_for_logger = AllLinksLogger(app)

naucse/models.py

@@ -83,10 +83,10 @@ def latex(self):
 
     @reify
     def css(self):
-        """ Returns lesson-specific extra CSS.
+        """Return lesson-specific extra CSS.
 
-        If the lesson defines extra css, the scope of the styles is limited to ``.lesson-content``,
-        a div which contains the actual lesson content.
+        If the lesson defines extra CSS, the scope of the styles is limited
+        to ``.lesson-content``, which contains the actual lesson content.
         """
         css = self.info.get("css")
 
@@ -200,7 +200,7 @@ def convert_url(url):
 
     @staticmethod
     def limit_css_to_lesson_content(css):
-        """ Returns ``css`` limited just to the ``.lesson-content`` element.
+        """Return ``css`` limited just to the ``.lesson-content`` element.
 
         This doesn't protect against malicious input.
         """
@@ -468,7 +468,7 @@ def _get_sessions(course, plan):
 
 
 class CourseMixin:
-    """ Couple of methods common for both :class:`Course` and :class:`CourseLink`.
+    """Methods common for both :class:`Course` and :class:`CourseLink`.
     """
 
     @reify
@@ -506,10 +506,13 @@ def __str__(self):
 
     data_filename = "info.yml"  # for MultipleModelDirProperty
 
-    # These two class attributes define what the function ``naucse.utils.forks:course_info`` returns from forks,
-    # meaning, the function in the fork looks at these lists that are in the fork and returns those.
-    # If you're adding an attribute to these lists, you have to make sure that you provide a default in
-    # the CourseLink attribute since the forks already forked will not be returning the value.
+    # These two class attributes define what the function
+    # ``naucse.utils.forks:course_info`` returns from forks,
+    # meaning, the function in the fork looks at these lists
+    # that are in the fork and returns those.
+    # If you're adding an attribute to these lists, you have to
+    # make sure that you provide a default in the CourseLink
+    # attribute since existing forks don't contain the value.
     COURSE_INFO = ["title", "description", "vars", "canonical"]
     RUN_INFO = ["title", "description", "start_date", "end_date", "canonical", "subtitle", "derives", "vars",
                 "default_start_time", "default_end_time"]
@@ -583,7 +586,7 @@ def optional_convert_time(timestr):
 
 
 class CourseLink(CourseMixin, Model):
-    """ A link to a course from a separate git repo.
+    """A link to a course from a separate git repo.
     """
 
     link = YamlProperty()
@@ -619,7 +622,9 @@ def base_course(self):
             return None
 
     def render(self, page_type, *args, **kwargs):
-        """ Renders a page in the fork, checks the content and registers urls to freeze.
+        """Render a page in the fork.
+
+        Check the content and registers URLs to freeze.
         """
         naucse.utils.views.forks_raise_if_disabled()
 
@@ -635,7 +640,8 @@ def render(self, page_type, *args, **kwargs):
             allowed_elements_parser.reset_and_feed(result.output["content"])
 
         if "urls" in result.output:
-            # freeze urls generated by the code in fork, but only if they start with the slug of the course
+            # Freeze URLs generated by the code in fork, but only if
+            # they start with the slug of the course
             absolute_urls_to_freeze.extend([url for url in result.output["urls"] if url.startswith(f"/{self.slug}/")])
 
         return result.output
@@ -662,12 +668,14 @@ def lesson_static(self, lesson_slug, path):
         return filename.parent, filename.name
 
     def get_footer_links(self, lesson_slug, page, **kwargs):
-        """ Returns links to previous page, to current session and to the next page. Each link
-            is either a dict with url and title keys or ``None``.
+        """Return links to previous page, current session and the next page.
+
+        Each link is either a dict with 'url' and 'title' keys or ``None``.
 
-            If :meth:`render_page` fails and a canonical versions is in the base repo, it's used instead
-            with a warning. This method provides the correct footer links for the page, since ``sessions``
-            is not included in the info provided by forks.
+        If :meth:`render_page` fails and a canonical version is in the
+        base repo, it's used instead with a warning.
+        This method provides the correct footer links for the page,
+        since ``sessions`` is not included in the info provided by forks.
         """
         naucse.utils.views.forks_raise_if_disabled()
 
@@ -728,7 +736,8 @@ def __str__(self):
 
 
 class MetaInfo:
-    """ Info about the current repository. """
+    """Info about the current repository.
+    """
 
     def __str__(self):
         return "Meta Information"
@@ -738,8 +747,10 @@ def __str__(self):
 
     @reify
     def slug(self):
-        """ Returns the slug of the repository based on the current branch. Returns the default if not on a branch,
-            the branch doesn't have a remote or the remote url can't be parsed.
+        """Return the slug of the repository based on the current branch.
+
+        Returns the default if not on a branch, the branch doesn't
+        have a remote, or the remote url can't be parsed.
         """
         from naucse.views import logger
 
@@ -780,7 +791,7 @@ def slug(self):
 
     @reify
     def branch(self):
-        """ Returns the active branch name or master if not on a branch.
+        """Return the active branch name, or 'master' if not on a branch.
         """
         from naucse.views import logger
 
@@ -831,9 +842,10 @@ def meta(self):
 
     @reify
     def safe_run_years(self):
-        # since even the basic info about the forked runs can be broken, we need to make sure the required info
-        # is provided. If ``RAISE_FORK_ERRORS`` is set, exceptions are raised here, otherwise the run is
-        # ignored completely.
+        # since even the basic info about the forked runs can be broken,
+        # we need to make sure the required info is provided.
+        # If ``RAISE_FORK_ERRORS`` is set, exceptions are raised here.
+        # Otherwise the run is ignored completely.
         safe_years = {}
         for year, run_years in self.run_years.items():
             safe_run_years = []

naucse/urlconverters.py

@@ -42,7 +42,8 @@ def to_url(self, value):
                 value = "course/"+value
             value = self.to_python(value)
 
-        if isinstance(value, dict):  # since the converter can be called with a dict mimicking a course
+        # the converter can be called with a dict mimicking a course
+        if isinstance(value, dict):
             return value["slug"]
 
         return value.slug
@@ -56,7 +57,8 @@ def to_url(self, value):
         if isinstance(value, Lesson):
             return value.slug
 
-        elif isinstance(value, dict):  # since the converter can be called with a dict mimicking a lesson
+        # the converter can be called with a dict mimicking a lesson
+        elif isinstance(value, dict):
             return value["slug"]
 
         return value
@@ -72,7 +74,8 @@ def to_python(self, value):
             abort(404)
 
     def to_url(self, value):
-        if isinstance(value, dict):  # since the converter can be called with a dict mimicking a lesson
+        # the converter can be called with a dict mimicking a lesson
+        if isinstance(value, dict):
             return value["slug"]
 
         if isinstance(value, str):

naucse/utils/forks.py

@@ -22,7 +22,9 @@ def get_course_from_slug(slug: str) -> Course:
 
 
 def course_info(slug: str, *args, **kwargs) -> Dict[str, Any]:
-    """ Returns info about the course/run. Returns some extra info when it's a run (based on COURSE_INFO/RUN_INFO)
+    """Return info about the given course.
+
+    Return some extra info when it's a run (based on COURSE_INFO/RUN_INFO)
     """
     course = get_course_from_slug(slug)
 
@@ -48,7 +50,7 @@ def course_info(slug: str, *args, **kwargs) -> Dict[str, Any]:
 
 
 def serialize_license(license) -> Optional[Dict[str, str]]:
-    """ Serializes a License instance into a dict
+    """Serialize a License instance into a dict.
     """
     if license:
         return {
@@ -60,7 +62,7 @@ def serialize_license(license) -> Optional[Dict[str, str]]:
 
 
 def render(page_type: str, slug: str, *args, **kwargs) -> Dict[str, Any]:
-    """ Returns a rendered page for a course, based on page_type and slug.
+    """Return a rendered page for a course, based on page_type and slug.
     """
     course = get_course_from_slug(slug)
 

naucse/utils/links.py

@@ -1,4 +1,4 @@
-""" These functions serve as validation and further processing of metadata from forks.
+"""Functions for validation and further processing of metadata from forks.
 """
 from xml.dom import SyntaxErr
 

naucse/utils/models.py

@@ -90,10 +90,13 @@ def compute(self, instance):
 
 
 class ForkProperty(LazyProperty):
-    """ Populated from the fork the model is pointing to.
+    """Populated from the fork the model is pointing to.
 
-    ``repo`` and ``branch`` indicate from which attribute of the instance the property should take info about the fork.
-    ``**kwargs`` are for `arca.Task` - the values can be callable (they get the instance as a parameter)
+    ``repo`` and ``branch`` indicate from which attribute of the instance
+    the property should take info about the fork.
+
+    ``**kwargs`` are for `arca.Task`. The values can be callable, in which
+    case they are called with the instance as a parameter.
     """
     def __init__(self, repo, branch, **kwargs):
         self.repo_prop = repo
@@ -158,8 +161,12 @@ def __get__(self, instance, cls):
 class DirProperty(LazyProperty):
     """Ordered dict of models from a subdirectory
 
-    If ``info.yml`` is present in the subdirectory, use it for the order
-    of the models.  The rest is appended alphabetically.
+    If ``info.yml`` is present in the subdirectory, it must contain a dict
+    with an "order" key, which is used to order the models.
+    The rest is appended in alphabetical order.
+
+    If ``keyfunc`` is given, it is used to create keys from directory names.
+    The default is ``str``, i.e. the directory name is used as dict key.
     """
     def __init__(self, cls, *subdir, keyfunc=str):
         self.cls = cls
@@ -190,16 +197,19 @@ def compute(self, instance):
 
 
 class MultipleModelDirProperty(DirProperty):
-    """ Ordered dict of models from a subdirectory.
+    """Ordered dict of models from a subdirectory.
 
     For directories that have different models inside of them.
-    The models which are supposed to be used are defined by files they load their content out of
-    and the definition prioritizes them - first come first serve. If none of the models is present in the folders
-    a exception is raised.
-    Each of the models has to have an attribute ``data_filename`` with the filename used for defining them.
 
-    The prioritization is important in forks - if the base repo is merged to forks and the definitions
-    would exclusive, info.yml and link.yml would be in conflict and further actions would be needed.
+    The models which are supposed to be used are defined by files they
+    load their content out of and the definition prioritizes them - first
+    come first serve.
+    If none of the models is present in the folders, an exception is raised.
+    Each of the models must have an attribute ``data_filename`` with
+    the filename used for defining them.
+
+    The prioritization is important in forks, since many directories
+    contain both info.yml and link.yml.
 
     Possible to order by ``info.yml`` just like DirProperty.
     """