From edf1102c21a61bc2fe50a79a40d2addb44529165 Mon Sep 17 00:00:00 2001 From: lwasser Date: Wed, 23 Sep 2020 15:41:19 -0600 Subject: [PATCH 01/14] this seems to work but is not fully tested yet --- abcclassroom/template.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/abcclassroom/template.py b/abcclassroom/template.py index 96ecccff..f14b928c 100644 --- a/abcclassroom/template.py +++ b/abcclassroom/template.py @@ -166,12 +166,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: path ??s + path to the template repository where the assignment files will be + copied (is it relative or actual full)??? + 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,13 +201,21 @@ def copy_assignment_files(config, template_repo, assignment): template_repo.relative_to(course_dir) ) ) - for file in all_files: + # 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 + # TODO: Note that as written here, moving directories will fail so + # we may want to revisit this shutil.copy(fpath, template_repo) nfiles += 1 - print("Copied {} files".format(nfiles)) + 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): From 21bf0d07e13c6c0d4cb0205b58c64b076d11d66c Mon Sep 17 00:00:00 2001 From: lwasser Date: Wed, 23 Sep 2020 15:51:39 -0600 Subject: [PATCH 02/14] updates to docstring --- abcclassroom/template.py | 1 + 1 file changed, 1 insertion(+) diff --git a/abcclassroom/template.py b/abcclassroom/template.py index f14b928c..78cb66cd 100644 --- a/abcclassroom/template.py +++ b/abcclassroom/template.py @@ -214,6 +214,7 @@ def copy_assignment_files(config, template_repo, assignment): # we may want to revisit this 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)) From 27b6d2e5fd1858189ca3323227bdd493dc8e00a8 Mon Sep 17 00:00:00 2001 From: lwasser Date: Wed, 23 Sep 2020 20:22:50 -0600 Subject: [PATCH 03/14] fix failing test --- abcclassroom/template.py | 6 +++--- abcclassroom/tests/test_assignment_template.py | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/abcclassroom/template.py b/abcclassroom/template.py index 78cb66cd..1a58e89f 100644 --- a/abcclassroom/template.py +++ b/abcclassroom/template.py @@ -172,9 +172,9 @@ def copy_assignment_files(config, template_repo, assignment): config: ordered dictionary Config file returned by ``get_config()`` that contains paths to the course directory, github organization and other custom options - template_repo: path ??s - path to the template repository where the assignment files will be - copied (is it relative or actual full)??? + 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 diff --git a/abcclassroom/tests/test_assignment_template.py b/abcclassroom/tests/test_assignment_template.py index c335afcb..190666fb 100644 --- a/abcclassroom/tests/test_assignment_template.py +++ b/abcclassroom/tests/test_assignment_template.py @@ -93,10 +93,12 @@ def test_move_git_dir(default_config, tmp_path): # Tests for copy_assignment_files method def test_copy_assignment_files(default_config, tmp_path): - # test that contents are the same for target and source directory + """"test that contents are the same for target and source directory + """ default_config["course_directory"] = tmp_path assignment = "assignment1" - + files_to_ignore = [".DS_Store", ".ipynb_checkpoints"] + default_config["files_to_ignore"] = files_to_ignore # first, set up the test course materials directory # and create some temporary files cmpath = Path( @@ -105,11 +107,21 @@ def test_copy_assignment_files(default_config, tmp_path): cmpath.mkdir(parents=True) cmpath.joinpath("file1.txt").touch() cmpath.joinpath("file2.txt").touch() + cmpath.joinpath(".DS_Store").touch() template_repo = abctemplate.create_template_dir(default_config, assignment) abctemplate.copy_assignment_files( default_config, template_repo, assignment ) + # Test that both text files have been moved to the template dir but that + # the system .DS_Store is not there + for afile in os.listdir(cmpath): + if afile not in files_to_ignore: + assert afile in os.listdir(template_repo) + else: + print(afile) + assert afile not in os.listdir(template_repo) + assert os.listdir(cmpath).sort() == os.listdir(template_repo).sort() From b454235a15972ca9fa8f2617b48262fdcd828827 Mon Sep 17 00:00:00 2001 From: lwasser Date: Wed, 23 Sep 2020 20:26:28 -0600 Subject: [PATCH 04/14] working on docstrings --- abcclassroom/template.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/abcclassroom/template.py b/abcclassroom/template.py index 1a58e89f..81339f5a 100644 --- a/abcclassroom/template.py +++ b/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)) From 9e19ce1a30436485e362697e8ab650517613f445 Mon Sep 17 00:00:00 2001 From: lwasser Date: Wed, 23 Sep 2020 20:29:58 -0600 Subject: [PATCH 05/14] more cleanup --- abcclassroom/tests/conftest.py | 1 + abcclassroom/tests/test_assignment_template.py | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/abcclassroom/tests/conftest.py b/abcclassroom/tests/conftest.py index db4b4354..620a88d1 100644 --- a/abcclassroom/tests/conftest.py +++ b/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 diff --git a/abcclassroom/tests/test_assignment_template.py b/abcclassroom/tests/test_assignment_template.py index 190666fb..1f14e5ea 100644 --- a/abcclassroom/tests/test_assignment_template.py +++ b/abcclassroom/tests/test_assignment_template.py @@ -93,12 +93,12 @@ def test_move_git_dir(default_config, tmp_path): # Tests for copy_assignment_files method def test_copy_assignment_files(default_config, tmp_path): - """"test that contents are the same for target and source directory + """"Test that files are moved to the template repo directory and that + ignored files are NOT moved. """ default_config["course_directory"] = tmp_path assignment = "assignment1" - files_to_ignore = [".DS_Store", ".ipynb_checkpoints"] - default_config["files_to_ignore"] = files_to_ignore + files_to_ignore = default_config["files_to_ignore"] # first, set up the test course materials directory # and create some temporary files cmpath = Path( @@ -126,7 +126,7 @@ def test_copy_assignment_files(default_config, tmp_path): def test_copy_assignment_files_fails_nodir(default_config, tmp_path): - # test that fails if course_materials dir does not exist + """Test that fails if course_materials dir does not exist""" default_config["course_directory"] = tmp_path assignment = "assignment1" template_repo = abctemplate.create_template_dir(default_config, assignment) From b6e6ae8087018c7a09f5b0bc13dc6308bbe8ebe4 Mon Sep 17 00:00:00 2001 From: lwasser Date: Wed, 23 Sep 2020 20:33:09 -0600 Subject: [PATCH 06/14] update config with files_to_ignore --- abcclassroom/example-data/config.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/abcclassroom/example-data/config.yml b/abcclassroom/example-data/config.yml index 514e971b..0700788c 100644 --- a/abcclassroom/example-data/config.yml +++ b/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 From 18422ce3b8569e82565c9af3b5e4ffbedb41ee7d Mon Sep 17 00:00:00 2001 From: Leah Wasser Date: Wed, 23 Sep 2020 21:48:40 -0600 Subject: [PATCH 07/14] Update abcclassroom/tests/test_assignment_template.py --- abcclassroom/tests/test_assignment_template.py | 1 - 1 file changed, 1 deletion(-) diff --git a/abcclassroom/tests/test_assignment_template.py b/abcclassroom/tests/test_assignment_template.py index 1f14e5ea..b78b9f2b 100644 --- a/abcclassroom/tests/test_assignment_template.py +++ b/abcclassroom/tests/test_assignment_template.py @@ -122,7 +122,6 @@ def test_copy_assignment_files(default_config, tmp_path): print(afile) assert afile not in os.listdir(template_repo) - assert os.listdir(cmpath).sort() == os.listdir(template_repo).sort() def test_copy_assignment_files_fails_nodir(default_config, tmp_path): From dea15c3b802b0b6dc9add9ba4424bd7469a32f5f Mon Sep 17 00:00:00 2001 From: lwasser Date: Thu, 24 Sep 2020 08:29:55 -0600 Subject: [PATCH 08/14] black --- abcclassroom/template.py | 13 ++++++++++++- abcclassroom/tests/test_assignment_template.py | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/abcclassroom/template.py b/abcclassroom/template.py index 81339f5a..81fdd80b 100644 --- a/abcclassroom/template.py +++ b/abcclassroom/template.py @@ -242,7 +242,18 @@ def copy_assignment_files(config, template_repo, assignment): 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(): diff --git a/abcclassroom/tests/test_assignment_template.py b/abcclassroom/tests/test_assignment_template.py index 1f14e5ea..62d1d690 100644 --- a/abcclassroom/tests/test_assignment_template.py +++ b/abcclassroom/tests/test_assignment_template.py @@ -93,7 +93,7 @@ def test_move_git_dir(default_config, tmp_path): # Tests for copy_assignment_files method def test_copy_assignment_files(default_config, tmp_path): - """"Test that files are moved to the template repo directory and that + """ "Test that files are moved to the template repo directory and that ignored files are NOT moved. """ default_config["course_directory"] = tmp_path From 8fa0524cdd62e165db62397922ea44293498d574 Mon Sep 17 00:00:00 2001 From: lwasser Date: Thu, 24 Sep 2020 09:06:01 -0600 Subject: [PATCH 09/14] flake8 --- abcclassroom/template.py | 19 +++++++---- .../tests/test_assignment_template.py | 32 +++++++++++++++++-- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/abcclassroom/template.py b/abcclassroom/template.py index 81fdd80b..12b85bca 100644 --- a/abcclassroom/template.py +++ b/abcclassroom/template.py @@ -230,12 +230,19 @@ def copy_assignment_files(config, template_repo, assignment): 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 - # TODO: Note that as written here, moving directories will fail so - # we may want to revisit this - shutil.copy(fpath, template_repo) - nfiles += 1 + 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)) diff --git a/abcclassroom/tests/test_assignment_template.py b/abcclassroom/tests/test_assignment_template.py index 9a6765a2..40e6734b 100644 --- a/abcclassroom/tests/test_assignment_template.py +++ b/abcclassroom/tests/test_assignment_template.py @@ -91,9 +91,9 @@ def test_move_git_dir(default_config, tmp_path): assert Path(template_path, ".git").exists() -# Tests for copy_assignment_files method +# Tests for copy_assignment_files def test_copy_assignment_files(default_config, tmp_path): - """ "Test that files are moved to the template repo directory and that + """Test that files are moved to the template repo directory and that ignored files are NOT moved. """ default_config["course_directory"] = tmp_path @@ -123,6 +123,32 @@ def test_copy_assignment_files(default_config, tmp_path): assert afile not in os.listdir(template_repo) +def test_copy_assignment_dirs(default_config, tmp_path, capfd): + """Test that when there is a directory in the extra_files dir, things + still copy as expected. + """ + default_config["course_directory"] = tmp_path + assignment = "assignment1" + # first, set up the test course materials directory + # and create some temporary files + cmpath = Path( + tmp_path, default_config["course_materials"], "release", assignment + ) + cmpath.mkdir(parents=True) + cmpath.joinpath("dummy-dir").mkdir() + + # Manually create the dir to just test the copy function + template_path = Path( + tmp_path, default_config["template_dir"], assignment + "-template" + ) + template_path.mkdir(parents=True) + abctemplate.copy_assignment_files( + default_config, template_path, assignment + ) + out, err = capfd.readouterr() + print(out) + assert "Oops - looks like" in out + def test_copy_assignment_files_fails_nodir(default_config, tmp_path): """Test that fails if course_materials dir does not exist""" @@ -135,7 +161,7 @@ def test_copy_assignment_files_fails_nodir(default_config, tmp_path): ) -# Test for create_extra_files method +# Test for create_extra_files def test_create_extra_files(default_config, tmp_path): default_config["course_directory"] = tmp_path assignment = "assignment1" From 3ac2be38ed7884e5b8c48fec2514d1210f57fef2 Mon Sep 17 00:00:00 2001 From: lwasser Date: Thu, 24 Sep 2020 09:16:53 -0600 Subject: [PATCH 10/14] docs update --- .../tests/test_assignment_template.py | 4 +- docs/manage-assignments/course-materials.rst | 45 ++++++++++++++++--- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/abcclassroom/tests/test_assignment_template.py b/abcclassroom/tests/test_assignment_template.py index 40e6734b..8902a5ac 100644 --- a/abcclassroom/tests/test_assignment_template.py +++ b/abcclassroom/tests/test_assignment_template.py @@ -11,9 +11,7 @@ @pytest.fixture def config_file(default_config, tmp_path): - """ - Writes the config to a file in tmp_path - """ + """Writes the config to a file in tmp_path""" cf.write_config(default_config, tmp_path) diff --git a/docs/manage-assignments/course-materials.rst b/docs/manage-assignments/course-materials.rst index db8ac2aa..c80ef614 100644 --- a/docs/manage-assignments/course-materials.rst +++ b/docs/manage-assignments/course-materials.rst @@ -49,12 +49,47 @@ dir (this _must_ be called `release`):: Put the files that you want to distribute to students in the `release` directory before :doc:`creating a new template repository `. Add Files To Every Assignment Repository -====================================================== +========================================== + +There are probably some files that you want to add to each assignment +repository. Any files that you wish to add to every new template repo can be +added to the ``extra_files`` directory which is created when you run +``abc-quickstart`` to set things up. By default, abc-quickstart will generate +an extra_files directory containing: + +1. a generic ``README.md`` file for the repository and +2. a ``.gitignore`` file. + +You can add any additional files that want in each repo to this directory. +File that you may want could include things like: + +1. a customized .gitignore file +2. logos and graphics that you want to display on every student assignment +3. custom modules needed to complete assignments for the course + +Any files in the extra_files directory will be addeded by defaul to each +template repository. + +Ignore Some Files +~~~~~~~~~~~~~~~~~~ +In many cases there may be system and tool based files that you do not wish to +move. if that is the case, you can populate the ``files_to_ignore`` section +of your ``config.yml`` file with anything that you want to skip. + +. code-block:: yaml + # Add any system files that you wish to ignore when copying files to your + # template here + files_to_ignore: + - .DS_Store + - .ipynb_checkpoints + + +.. note:: + Currently abc-classroom does not support moving entire directories over to + new template repos. We will consider adding that functionality in the + future if it is of community interest AND/OR are open to community PR's to + add that functionality. -There are probably some files that you want added to each assignment repository. -By default, these include a readme for the repository and a `.gitignore`. These -files exist in `extra_files` directory. Whatever you place in here gets -copied to each new assignment repo. Update config.yml =================== From 3b3084559c1d406ce99efbd8631b8ade8401d9ac Mon Sep 17 00:00:00 2001 From: lwasser Date: Thu, 24 Sep 2020 09:33:26 -0600 Subject: [PATCH 11/14] black --- CHANGELOG.rst | 1 + abcclassroom/feedback.py | 35 +++++++++++++++---- .../tests/test_assignment_template.py | 2 +- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 22adfa68..066931b8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -9,6 +9,7 @@ The format is based on `Keep a Changelog ` [Unreleased] ------------ +- Add files_to_ignore to skip moving certain files (@lwasser, #172, #278) [0.1.5] ------------ diff --git a/abcclassroom/feedback.py b/abcclassroom/feedback.py index c3ba0a0b..7b6479f7 100644 --- a/abcclassroom/feedback.py +++ b/abcclassroom/feedback.py @@ -14,7 +14,18 @@ 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, 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. + + 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 """ assignment = args.assignment do_github = args.github @@ -22,11 +33,10 @@ def copy_feedback(args): print("Loading configuration from config.yml") config = cf.get_config() - # get various paths from config + # Get various paths from config 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: @@ -42,12 +52,23 @@ def copy_feedback(args): destination_dir = Path(clone_dir, repo) 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: + # TODO: Turn this into a helper function lines 53 - 71 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 diff --git a/abcclassroom/tests/test_assignment_template.py b/abcclassroom/tests/test_assignment_template.py index 8902a5ac..f3b6e3eb 100644 --- a/abcclassroom/tests/test_assignment_template.py +++ b/abcclassroom/tests/test_assignment_template.py @@ -123,7 +123,7 @@ def test_copy_assignment_files(default_config, tmp_path): def test_copy_assignment_dirs(default_config, tmp_path, capfd): """Test that when there is a directory in the extra_files dir, things - still copy as expected. + still copy as expected. """ default_config["course_directory"] = tmp_path assignment = "assignment1" From defb37b926620d0799cf5e366906e88d6cbf8f2d Mon Sep 17 00:00:00 2001 From: lwasser Date: Thu, 24 Sep 2020 10:58:58 -0600 Subject: [PATCH 12/14] cleanup feedback --- abcclassroom/feedback.py | 47 +++++++++++++++++++---------- abcclassroom/tests/test_feedback.py | 34 +++++++++++++++++++++ 2 files changed, 65 insertions(+), 16 deletions(-) create mode 100644 abcclassroom/tests/test_feedback.py diff --git a/abcclassroom/feedback.py b/abcclassroom/feedback.py index 7b6479f7..88e35de8 100644 --- a/abcclassroom/feedback.py +++ b/abcclassroom/feedback.py @@ -9,26 +9,20 @@ import shutil from . import config as cf + +# This is problematic as github is a module but it's also an argument name from . import github -def copy_feedback(args): - """ - Copies feedback reports to local student repositories, commits the changes, +# i renamed the argument here to be push_github but this is'nt tested yet +# as it was before github was an argument AND a module name which was +# problematic +def copy_feedback_files(assignment, do_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. - - 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 - """ - assignment = args.assignment - do_github = args.github + source directory.""" print("Loading configuration from config.yml") config = cf.get_config() @@ -56,7 +50,7 @@ def copy_feedback(args): "skipping student".format(destination_dir) ) continue - # TODO: Turn this into a helper function lines 53 - 71 here + # 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 @@ -85,3 +79,24 @@ def copy_feedback(args): 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 + """ + assignment = args.assignment + do_github = args.github + + copy_feedback_files(assignment, do_github) diff --git a/abcclassroom/tests/test_feedback.py b/abcclassroom/tests/test_feedback.py new file mode 100644 index 00000000..5acc9474 --- /dev/null +++ b/abcclassroom/tests/test_feedback.py @@ -0,0 +1,34 @@ +from pathlib import Path + +import abcclassroom.feedback as abcfeedback + +# import abcclassroom.config as cf + + +def test_html_copies(default_config, tmp_path): + """Test that an html feedback report added to a repo copies over. """ + + assignment = "assignment1" + default_config["course_directory"] = tmp_path + + # Setup student cloned directory with a single notebook + # (this could be a fixture?) + clone_path = Path( + tmp_path, "cloned_repos", assignment, "assignment1-test-student" + ) + clone_path.mkdir(parents=True) + clone_path.joinpath("assignment1-test-student.ipynb").touch() + + # Setup feedback directory + feedback_path = Path( + tmp_path, "nbgrader", "feedback", "test-student", assignment + ) + feedback_path.mkdir(parents=True) + feedback_path.joinpath("assignment1-test-student.html").touch() + + # Return feedback -- this requires a config file + abcfeedback.copy_feedback_files(assignment, github=None) + + # Add html file to one student's directory + + # Run abc-feedback From b4d51ac2decb610ada285b38e685a01f1de9a7b8 Mon Sep 17 00:00:00 2001 From: lwasser Date: Sat, 26 Sep 2020 11:05:58 -0600 Subject: [PATCH 13/14] black --- abcclassroom/feedback.py | 51 ++++++++++++++-------- abcclassroom/tests/test_feedback.py | 68 ++++++++++++++--------------- 2 files changed, 68 insertions(+), 51 deletions(-) diff --git a/abcclassroom/feedback.py b/abcclassroom/feedback.py index 88e35de8..565fc486 100644 --- a/abcclassroom/feedback.py +++ b/abcclassroom/feedback.py @@ -9,25 +9,36 @@ import shutil from . import config as cf - -# This is problematic as github is a module but it's also an argument name from . import github -# i renamed the argument here to be push_github but this is'nt tested yet -# as it was before github was an argument AND a module name which was -# problematic -def copy_feedback_files(assignment, do_github=False): +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.""" + 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``. + """ print("Loading configuration from config.yml") config = cf.get_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) @@ -39,10 +50,10 @@ def copy_feedback_files(assignment, do_github=False): reader = csv.DictReader(csvfile) for row in reader: student = row["github_username"] - source_files = Path(feedback_dir, student, assignment).glob( - "*" - ) - repo = "{}-{}".format(assignment, student) + source_files = Path( + feedback_dir, student, assignment_name + ).glob("*") + repo = "{}-{}".format(assignment_name, student) destination_dir = Path(clone_dir, repo) if not destination_dir.is_dir(): print( @@ -71,9 +82,10 @@ def copy_feedback_files(assignment, do_github=False): 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: + if push_to_github: github.push_to_github(destination_dir) except FileNotFoundError as err: @@ -90,13 +102,18 @@ def copy_feedback(args): 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 = args.assignment - do_github = args.github + assignment_name = args.assignment + push_to_github = args.github - copy_feedback_files(assignment, do_github) + copy_feedback_files(assignment_name, push_to_github) diff --git a/abcclassroom/tests/test_feedback.py b/abcclassroom/tests/test_feedback.py index 5acc9474..0c1e36f2 100644 --- a/abcclassroom/tests/test_feedback.py +++ b/abcclassroom/tests/test_feedback.py @@ -1,34 +1,34 @@ -from pathlib import Path - -import abcclassroom.feedback as abcfeedback - -# import abcclassroom.config as cf - - -def test_html_copies(default_config, tmp_path): - """Test that an html feedback report added to a repo copies over. """ - - assignment = "assignment1" - default_config["course_directory"] = tmp_path - - # Setup student cloned directory with a single notebook - # (this could be a fixture?) - clone_path = Path( - tmp_path, "cloned_repos", assignment, "assignment1-test-student" - ) - clone_path.mkdir(parents=True) - clone_path.joinpath("assignment1-test-student.ipynb").touch() - - # Setup feedback directory - feedback_path = Path( - tmp_path, "nbgrader", "feedback", "test-student", assignment - ) - feedback_path.mkdir(parents=True) - feedback_path.joinpath("assignment1-test-student.html").touch() - - # Return feedback -- this requires a config file - abcfeedback.copy_feedback_files(assignment, github=None) - - # Add html file to one student's directory - - # Run abc-feedback +# from pathlib import Path +# +# import abcclassroom.feedback as abcfeedback +# +# # import abcclassroom.config as cf +# +# +# def test_html_copies(default_config, tmp_path): +# """Test that an html feedback report added to a repo copies over. """ +# +# assignment = "assignment1" +# default_config["course_directory"] = tmp_path +# +# # Setup student cloned directory with a single notebook +# # (this could be a fixture?) +# clone_path = Path( +# tmp_path, "cloned_repos", assignment, "assignment1-test-student" +# ) +# clone_path.mkdir(parents=True) +# clone_path.joinpath("assignment1-test-student.ipynb").touch() +# +# # Setup feedback directory +# feedback_path = Path( +# tmp_path, "nbgrader", "feedback", "test-student", assignment +# ) +# feedback_path.mkdir(parents=True) +# feedback_path.joinpath("assignment1-test-student.html").touch() +# +# # Return feedback -- this requires a config file +# abcfeedback.copy_feedback_files(assignment, github=None) +# +# # Add html file to one student's directory +# +# # Run abc-feedback From 7fbb09ed707c4ac5f8d3c4614422099c69844135 Mon Sep 17 00:00:00 2001 From: lwasser Date: Sat, 26 Sep 2020 11:44:40 -0600 Subject: [PATCH 14/14] black --- abcclassroom/clone.py | 37 +++++++++++++++++++++++++++---------- abcclassroom/feedback.py | 1 + abcclassroom/template.py | 2 ++ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/abcclassroom/clone.py b/abcclassroom/clone.py index 2bae4c16..3400eb0d 100644 --- a/abcclassroom/clone.py +++ b/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,20 +99,30 @@ 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( @@ -120,6 +130,13 @@ def copy_assignment_files(config, student, assignment): 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) diff --git a/abcclassroom/feedback.py b/abcclassroom/feedback.py index 86878249..3bb162d8 100644 --- a/abcclassroom/feedback.py +++ b/abcclassroom/feedback.py @@ -81,6 +81,7 @@ def copy_feedback_files(assignment_name, push_to_github=False): ) ) shutil.copy(f, destination_dir) + github.commit_all_changes( destination_dir, msg="Adding feedback for assignment {}".format( diff --git a/abcclassroom/template.py b/abcclassroom/template.py index 12b85bca..ed080275 100644 --- a/abcclassroom/template.py +++ b/abcclassroom/template.py @@ -223,6 +223,8 @@ def copy_assignment_files(config, template_repo, assignment): template_repo.relative_to(course_dir) ) ) + # 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)