Move cloned student directories into assignment subdirectory (#288)

* syntax cleanup

* fix docstring

* black

* black

* update changelog and fix manifest issue

Author: Leah Wasser

CHANGELOG.rst

@@ -9,6 +9,9 @@ The format is based on `Keep a Changelog <https://keepachangelog.com/en/1.0.0/>`
 
 [Unreleased]
 ------------
+- 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)
 
 [0.1.5]
 ------------

MANIFEST.in

@@ -1,3 +1,3 @@
-graft abcclassroom/example-files
+graft abcclassroom/example-data
 include README.md
 include LICENSE.rst

abcclassroom/clone.py

@@ -17,6 +17,17 @@ def clone_or_update_repo(organization, repo, clone_dir, skip_existing):
     Tries to clone the single repository 'repo' from the organization. If the
     local repository already exists, pulls instead of cloning (unless the
     skip flag is set, in which case it does nothing).
+
+    Parameters
+    ----------
+    organization : string
+        Organization where your GitHub classroom lives.
+    repo : string
+        Name of the student's GitHub repo.
+    clone_dir : string
+        Name of the clone directory.
+    skip_existing : boolean
+        True if you wish to skip copying files to existing repos.
     """
     destination_dir = Path(clone_dir, repo)
     if destination_dir.is_dir():
@@ -56,7 +67,8 @@ def clone_student_repos(args):
             "repositories but will not copy any assignment files."
         )
     try:
-        Path(course_dir, clone_dir).mkdir(exist_ok=True)
+        # Create the assignment subdirectory path and ensure it exists
+        Path(course_dir, clone_dir, assignment).mkdir(exist_ok=True)
         missing = []
         with open(roster_filename, newline="") as csvfile:
             reader = csv.DictReader(csvfile)
@@ -66,7 +78,10 @@ def clone_student_repos(args):
                 repo = "{}-{}".format(assignment, student)
                 try:
                     clone_or_update_repo(
-                        organization, repo, clone_dir, skip_existing
+                        organization,
+                        repo,
+                        Path(clone_dir, assignment),
+                        skip_existing,
                     )
                     if materials_dir is not None:
                         copy_assignment_files(config, student, assignment)
@@ -80,7 +95,7 @@ def clone_student_repos(args):
                 print(" {}".format(r))
 
     except FileNotFoundError as err:
-        print("Cannot find roster file".format(roster_filename))
+        print("Cannot find roster file: {}".format(roster_filename))
         print(err)
 
 
@@ -91,7 +106,11 @@ def copy_assignment_files(config, student, assignment):
     materials_dir = cf.get_config_option(config, "course_materials", False)
     clone_dir = cf.get_config_option(config, "clone_dir", True)
     repo = "{}-{}".format(assignment, student)
-    files = Path(course_dir, clone_dir, repo).glob("*.ipynb")
+
+    # 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")
     destination = Path(
         course_dir, materials_dir, "submitted", student, assignment
     )

abcclassroom/feedback.py

@@ -14,9 +14,15 @@
 
 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.
+    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.
+
+    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.
     """
-    assignment = args.assignment
+    assignment_name = args.assignment
     do_github = args.github
 
     print("Loading configuration from config.yml")
@@ -26,7 +32,6 @@ def copy_feedback(args):
     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)
-    organization = cf.get_config_option(config, "organization", True)
     materials_dir = cf.get_config_option(config, "course_materials", True)
 
     try:
@@ -35,16 +40,16 @@ def copy_feedback(args):
             reader = csv.DictReader(csvfile)
             for row in reader:
                 student = row["github_username"]
-                source_files = Path(feedback_dir, student, assignment).glob(
-                    "*"
-                )
-                repo = "{}-{}".format(assignment, student)
-                destination_dir = Path(clone_dir, repo)
+                source_files = Path(
+                    feedback_dir, student, assignment_name
+                ).glob("*")
+                repo_name = "{}-{}".format(assignment_name, student)
+                # The repos now live in clone_dir/assignment-name/repo-name
+                destination_dir = Path(clone_dir, assignment_name, repo_name)
                 if not destination_dir.is_dir():
                     print(
-                        "Local student repository {} does not exist; skipping student".format(
-                            destination_dir
-                        )
+                        "Local student repository {} does not exist; skipping "
+                        "student".format(destination_dir)
                     )
                     continue
                 for f in source_files:
@@ -56,7 +61,9 @@ def copy_feedback(args):
                     shutil.copy(f, destination_dir)
                 github.commit_all_changes(
                     destination_dir,
-                    msg="Adding feedback for assignment {}".format(assignment),
+                    msg="Adding feedback for assignment {}".format(
+                        assignment_name
+                    ),
                 )
                 if do_github:
                     github.push_to_github(destination_dir)

abcclassroom/tests/test_clone.py

@@ -1,15 +1,14 @@
 # Tests for clone script
 
 import pytest
-import os
 from pathlib import Path
 
 import abcclassroom.clone as abcclone
-import abcclassroom.github as github
 
+# TODO - this should be a fixture
 test_data = {
     "assignment": "assignment1",
-    "students": ["bert"],
+    "students": ["bert", "alana"],
     "files": ["nb1.ipynb", "nb2.ipynb", "junk.csv"],
 }
 
@@ -27,8 +26,8 @@ def test_files(default_config, tmp_path):
     files = test_data["files"]
     for s in students:
         repo_name = "{}-{}".format(assignment, s)
-        assignment_path = Path(tmp_path, clone_dir, repo_name)
-        assignment_path.mkdir(parents=True)
+        assignment_path = Path(tmp_path, clone_dir, assignment, repo_name)
+        assignment_path.mkdir(parents=True, exist_ok=True)
         for f in files:
             Path(assignment_path, f).touch()
 
@@ -58,5 +57,5 @@ def test_copy_assignment_files(default_config, test_files):
             Path(
                 materials_dir, "submitted", s, assignment, "junk.csv"
             ).exists()
-            == False
+            is False
         )