Add files_to_ignore to ensure that only files that we want to copy ov…

…er are actually moved (#289)

* this seems to work but is not fully tested yet

* updates to docstring

* fix failing test

* working on docstrings

* more cleanup

* update config with files_to_ignore

* Update abcclassroom/tests/test_assignment_template.py

* black

* flake8

* docs update

* black

* cleanup feedback

* black

* black

CHANGELOG.rst

@@ -9,6 +9,7 @@ The format is based on `Keep a Changelog <https://keepachangelog.com/en/1.0.0/>`
 
 [Unreleased]
 ------------
+- Add files_to_ignore to skip moving certain files (@lwasser, #172, #278)
 - Clone dir copies assignments into an assignment-name dir rather than cloned
   dir root (@lwasser, #276)
 - Fix manifest file to bundle example-data not example-files dir (@lwasser, #272 take two)

abcclassroom/clone.py

@@ -50,7 +50,7 @@ def clone_student_repos(args):
     notebook files into the 'course_materials/submitted' directory, based on
     course_materials set in config.yml."""
 
-    assignment = args.assignment
+    assignment_name = args.assignment
     skip_existing = args.skip_existing
 
     print("Loading configuration from config.yml")
@@ -68,23 +68,23 @@ def clone_student_repos(args):
         )
     try:
         # Create the assignment subdirectory path and ensure it exists
-        Path(course_dir, clone_dir, assignment).mkdir(exist_ok=True)
+        Path(course_dir, clone_dir, assignment_name).mkdir(exist_ok=True)
         missing = []
         with open(roster_filename, newline="") as csvfile:
             reader = csv.DictReader(csvfile)
             for row in reader:
                 student = row["github_username"]
                 # expected columns: identifier,github_username,github_id,name
-                repo = "{}-{}".format(assignment, student)
+                repo = "{}-{}".format(assignment_name, student)
                 try:
                     clone_or_update_repo(
                         organization,
                         repo,
-                        Path(clone_dir, assignment),
+                        Path(clone_dir, assignment_name),
                         skip_existing,
                     )
                     if materials_dir is not None:
-                        copy_assignment_files(config, student, assignment)
+                        copy_assignment_files(config, student, assignment_name)
                 except RuntimeError:
                     missing.append(repo)
         if len(missing) == 0:
@@ -99,27 +99,44 @@ def clone_student_repos(args):
         print(err)
 
 
-def copy_assignment_files(config, student, assignment):
+def copy_assignment_files(config, student, assignment_name):
     """Copies all notebook files from clone_dir to course_materials/submitted.
-    Will overwrite any existing files with the same name."""
+    Will overwrite any existing files with the same name.
+
+    Parameters
+    -----------
+    config: dict
+        config file returned as a dictionary from get_config()
+    student:
+    assignment_name: string
+        Name of the assignment for which files are being copied
+
+    """
     course_dir = cf.get_config_option(config, "course_directory", True)
     materials_dir = cf.get_config_option(config, "course_materials", False)
     clone_dir = cf.get_config_option(config, "clone_dir", True)
-    repo = "{}-{}".format(assignment, student)
+    repo = "{}-{}".format(assignment_name, student)
 
     # Copy files from the cloned_dirs/assignment name directory
     # TODO - right now this ONLY copies notebooks but we may want to copy
     # other file types like .py files as well.
-    files = Path(course_dir, clone_dir, assignment, repo).glob("*.ipynb")
+    files = Path(course_dir, clone_dir, assignment_name, repo).glob("*.ipynb")
     destination = Path(
-        course_dir, materials_dir, "submitted", student, assignment
+        course_dir, materials_dir, "submitted", student, assignment_name
     )
     destination.mkdir(parents=True, exist_ok=True)
     print(
         "Copying files from {} to {}".format(
             Path(clone_dir, repo), destination
         )
     )
+    # We are copying files here source: clone dir -> nbgrader submitted
+    # TODO: use the copy files helper - in this case it's only copying .ipynb
+    # files
+    # but i could see someone wanting to copy other types of files such as .py
+    # So it may make sense to implement a copy files helper here as well even
+    # tho it's adding a bit of additional steps - it's still a very small
+    # operation
     for f in files:
         print("copying {} to {}".format(f, destination))
         copy(f, destination)

abcclassroom/example-data/config.yml

@@ -29,3 +29,11 @@ clone_dir: cloned_repos
 # course_dir unless you enter an absolute path (i.e. starting with '/' on
 # Linux or OS X or with 'C:' on Windows).
 template_dir: assignment_repos
+
+# A list of files that you do not want to be copied from your assignment
+# release directory to the github template repo. These may be system files
+# Checkpoints or other files that are created by various tools and operating
+# system functions. Add as many as you wish to the list below!
+files_to_ignore:
+- .DS_Store
+- .ipynb_checkpoints

abcclassroom/feedback.py

@@ -12,23 +12,33 @@
 from . import github
 
 
-def copy_feedback(args):
-    """
-    Copies feedback reports to local student repositories and commits the
-    changes. If the --github flag is used, it also will push the changes to
-    GitHub.
+def copy_feedback_files(assignment_name, push_to_github=False):
+    """Copies feedback reports to local student repositories, commits the
+    changes,
+    and (optionally) pushes to github. Assumes files are in the directory
+    course_materials/feedback/student/assignment. Copies all files in the
+    source directory.
 
-    Assumes feedback files are in the directory
-    course_materials/feedback/student/assignment-name following a typical
-    nbgrader structure. Copies all files in the source directory.
+    Parameters
+    -----------
+    assignment_name: string
+        Name of the assignment for which feedback is being processed.
+    push_to_github: boolean (default = False)
+        ``True`` if you want to automagically push feedback to GitHub after
+        committing changes
+
+    Returns
+    -------
+        Moves feedback html files from the student feedback directory to the
+        cloned_repos directory. Will commit to git and if ``push_to_github``
+        is ``True``, it will also run ``git push``.
     """
-    assignment_name = args.assignment
-    do_github = args.github
 
     print("Loading configuration from config.yml")
     config = cf.get_config()
 
-    # get various paths from config
+    # Get various paths from config
+    # I think we do this a bunch so is it worth a helper for it?
     roster_filename = cf.get_config_option(config, "roster", True)
     course_dir = cf.get_config_option(config, "course_directory", True)
     clone_dir = cf.get_config_option(config, "clone_dir", True)
@@ -52,22 +62,61 @@ def copy_feedback(args):
                         "student".format(destination_dir)
                     )
                     continue
-                for f in source_files:
+                # TODO: Turn this into a helper function lines 46 - 64 here
+                # Don't copy any system related files -- not this is exactly
+                # the same code used in the template.py copy files function.
+                # this could become a helper that just moves files. I think
+                # we'd call it a few times so it's worth doing... and when /
+                # if we add support to move directories we could just add it
+                # in one place.
+                files_to_ignore = cf.get_config_option(
+                    config, "files_to_ignore", True
+                )
+                files_to_move = set(source_files).difference(files_to_ignore)
+
+                for f in files_to_move:
                     print(
                         "Copying {} to {}".format(
                             f.relative_to(course_dir), destination_dir
                         )
                     )
                     shutil.copy(f, destination_dir)
+
                 github.commit_all_changes(
                     destination_dir,
                     msg="Adding feedback for assignment {}".format(
                         assignment_name
                     ),
                 )
-                if do_github:
+                if push_to_github:
                     github.push_to_github(destination_dir)
 
     except FileNotFoundError as err:
         print("Missing file or directory:")
         print(" ", err)
+
+
+def copy_feedback(args):
+    """
+    Copies feedback reports to local student repositories, commits the changes,
+    and (optionally) pushes to github. Assumes files are in the directory
+    course_materials/feedback/student/assignment. Copies all files in the
+    source directory. This is the command line implementation.
+
+    Parameters
+    ----------
+    args: string
+        Command line argument for the copy_feedback function. Options include:
+        assignment: Assignment name
+        github: a flag to push to GitHub vs only commit the change
+
+    Returns
+    -------
+        Moves feedback html files from the student feedback directory to the
+        cloned_repos directory. Will commit to git and if ``push_to_github``
+        is ``True``, it will also run ``git push``.
+    """
+    assignment_name = args.assignment
+    push_to_github = args.github
+
+    copy_feedback_files(assignment_name, push_to_github)

abcclassroom/template.py

@@ -22,7 +22,12 @@ def new_update_template(args):
 
     Creates an assignment entry in the config file if one does not already
     exist.
+
+    Parameters
+    ----------
+    args : command line arguments
     """
+
     print("Loading configuration from config.yml")
     config = cf.get_config()
 
@@ -61,6 +66,23 @@ def new_update_template(args):
 def create_or_update_remote(
     template_repo_path, organization, repo_name, token
 ):
+    """
+    Push template repo to github creating a new repository or update the
+    repo's contents
+
+    Parameters
+    ----------
+    template_repo_path : string
+        The path to the template repo on your local computer.
+    organization : string
+        The name of the organization where your GitHub Classroom lives.
+    repo_name : string
+        The name of the template repository to create on GitHub
+    token : github token
+        Used to authenticate with GitHub via the API. Created by running
+        ``abc-init``
+
+    """
     remote_exists = github.remote_repo_exists(organization, repo_name, token)
     if not remote_exists:
         print("Creating remote repo {}".format(repo_name))
@@ -166,12 +188,25 @@ def create_template_dir(config, assignment, mode="fail"):
 def copy_assignment_files(config, template_repo, assignment):
     """Copy all of the files from the course_materials/release directory for the
     assignment into the template repo directory.
+
+    Parameters
+    ----------
+    config: ordered dictionary
+        Config file returned by ``get_config()`` that contains paths to the
+        course directory, github organization and other custom options
+    template_repo: os path object
+        Absolute path to the template repository where the assignment files
+        will be copied
+    assignment: string
+        name of the assignment being copied
+
     """
 
     course_dir = cf.get_config_option(config, "course_directory", True)
     materials_dir = cf.get_config_option(config, "course_materials", True)
     parent_path = utils.get_abspath(materials_dir, course_dir)
     release_dir = Path(parent_path, "release", assignment)
+
     if not release_dir.is_dir():
         print(
             "release directory {} does not exist; exiting\n".format(
@@ -188,17 +223,46 @@ def copy_assignment_files(config, template_repo, assignment):
             template_repo.relative_to(course_dir)
         )
     )
-    for file in all_files:
+    # TODO this could also use the copy files helper - thinking to put it in
+    # the utils module
+    # Get a list of files to ignore - maybe our default config has some
+    # could have some defaults - then remove all files that we want to ignore
+    files_to_ignore = cf.get_config_option(config, "files_to_ignore", True)
+    files_to_move = set(all_files).difference(files_to_ignore)
+
+    for file in files_to_move:
         fpath = Path(release_dir, file)
-        print(" {}".format(fpath.relative_to(course_dir)))
-        # overwrites if fpath exists in template_repo
-        shutil.copy(fpath, template_repo)
-        nfiles += 1
-    print("Copied {} files".format(nfiles))
+        if fpath.is_dir():
+            # TODO: Note that as written here, moving directories will fail so
+            print(
+                "Oops - looks like {} is a directory. Currently I can't "
+                "move that for you. Contact the abc-classroom maintainers"
+                "if this is a feature that you'd "
+                "like".format(fpath.relative_to(course_dir))
+            )
+        else:
+            print(" {}".format(fpath.relative_to(course_dir)))
+            # Overwrites if fpath exists in template_repo
+            shutil.copy(fpath, template_repo)
+            nfiles += 1
+
+    print("Copied {} files to your assignment directory!".format(nfiles))
+    print("The files copied include: {}".format(files_to_move))
 
 
 def create_extra_files(config, template_repo, assignment):
-    """Copy any extra files that exist the extra_files directory """
+    """Copy any extra files that exist the extra_files directory
+
+    Parameters
+    ----------
+    config : Path
+        Path to the config.yml file??
+    template_repo : Path object ?
+        Path to the template repo that you wish to copy files over to. ??
+    assignment : string
+        Name of the assignment that you want to copy files over for.
+
+    """
     course_dir = cf.get_config_option(config, "course_directory", True)
     extra_path = Path(course_dir, "extra_files")
     if extra_path.is_dir():

abcclassroom/tests/conftest.py

@@ -14,5 +14,6 @@ def default_config():
         "template_dir": "test_template",
         "course_materials": "nbgrader",
         "clone_dir": "cloned-repos",
+        "files_to_ignore": [".DS_Store", ".ipynb_checkpoints"],
     }
     return config