jupyter / nbgrader

A system for assigning and grading notebooks
https://nbgrader.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
1.28k stars 316 forks source link

nbgrader validation fails accessing external files #894

Closed johnhw closed 6 years ago

johnhw commented 6 years ago

Operating system

Windows 10 64 bit

nbgrader --version

Python version 3.6.1 |Anaconda 4.4.0 (64-bit)| (default, May 11 2017, 13:25:24) [MSC v.1900 64 bit (AMD64)] nbgrader version 0.5.3

jupyterhub --version (if used with JupyterHub)

N/A

Notebook - Help, About version info

The version of the notebook server is 5.0.0

Expected behavior

An exercise notebook reads an external file (e.g. data/data.txt) and autograding tests depend on reading this file. I edit the source/ version before distributing. All tests pass when running the notebook with Run All. I hit Validate and expect to see the same result.

Actual behavior

If a notebook accesses an external file, validation will fail if Jupyter was launched from the root of the nbgrader directory (or anywhere except the specific directory where the notebook is). This is because Validate seems to set/(or be run from) the current directory to the launch dir of Jupyter.

This also happens with the release version (validation will fail even if the solution is correct, because the external files will not be accessible from the path nbgrader validate is run).

Steps to reproduce the behavior

Create a new assignment. Add any file (e.g. "data/data.txt"). It doesn't have to be in a subdirectory for this to happen. Add an autograder answer and test like:

  ### BEGIN SOLUTION
  with open("data/data.txt") as f:
         contents = f.read()
  ### END SOLUTION

  # test
  # will fail because contents will not be defined
  assert(len(contents)>0)

Alternatively, just create an autograder test:

 import os
 raise(Exception(os.getcwd()))

Hit Validate The current working directory will be the launch directory of Jupyter, and not the notebook directory.

I can't see an obvious workaround for this. Apologies if I am missing something obvious.

johnhw commented 6 years ago

I have fixed this (apparently) by adding a chdir() context manager (copied from server_extensions/validate_assignment/handlers.py) around the Validator.validate() method of validate.py, so that it reads:

def validate(self, filename):
        self.log.info("Validating '{}'".format(os.path.abspath(filename)))        
        with chdir(os.path.dirname(filename)):            
             nb = self._preprocess(os.path.basename(filename))
             ...

I'm not sure if that is bad idea for other reasons, but it seems to do what I expect now.

flareau commented 6 years ago

I have exactly that problem. Do you have the same bug when autograding as well?

johnhw commented 6 years ago

No, I don't have this problem with autograding. Access to external files works correctly in that case, using the same test exercise that fails validation. It's just validation which seems to break.

flareau commented 6 years ago

Validating from the command line worked without a glitch for me, while it failed as you described with the web interface.

On Sep 29, 2017 at 03:51, <johnhw (mailto:notifications@github.com)> wrote:

No, I don't have this problem with autograding. Access to external files works correctly in that case, using the same test exercise that fails validation. It's just validation which seems to break.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub (https://github.com/jupyter/nbgrader/issues/894#issuecomment-333057239), or mute the thread (https://github.com/notifications/unsubscribe-auth/ANmxdGEsb2OnO3N68n26tpUbyHERMIsyks5snKGHgaJpZM4PiBKr).

johnhw commented 6 years ago

nbgrader validate <notebook_path> will fail for me unless run in the directory where the notebook is present.

     nbgrader validate lab.ipynb       # works
     nbgbrader validate source/lab1/lab.ipynb   # does not work, for the same reason as above

The fix I posted above also resolves this problem.

johnhw commented 6 years ago

This was already fixed in #880 Oops.