Compatibility with nbconvert v6

[jhamrick]

Following on @rkdarst 's initial PR in #1405, this updates nbgrader to be compatible with nbconvert version 6+. The main changes are:

• Adhere to the new templating system in nbconvert.
• Remove execution info from notebook metadata, as this breaks tests.
• Fix overwritten parts of the ExecutePreprocessor.
• Unpin the dependency on nbconvert v5.

Closes: #1373, Closes: #1382, Closes: #1367, Closes #1411

[jhamrick]

cc @ttimbers @nthiery

[jhamrick]

Windows tests are failing. Seems like it possibly has to do with the execute preprocessor. I am not sure if this is transient or caused by the upgrade to nbconvert but will try to investigate later...

[echuber2]

It looks like there were breaking changes on Windows + Python 3.8 related to async IO. Some related threads:

https://github.com/tornadoweb/tornado/issues/2608
https://github.com/mkdocs/mkdocs/issues/1885
https://github.com/tornadoweb/tornado/issues/2751

[jhamrick]

Thanks @echuber2 , indeed it looks as though this is due to asyncio issues on windows. The proper fix will have to com from nbclient (jupyter/nbclient#85) but for now I'll see if I can use the workaround.

[jhamrick]

Ok, the workaround works, but the nbextensions are still failing the tests for some reason. I tried, and failed, to resurrect my windows dev environment, so I'm blocked on figuring that out for the time being. I'll try to spin up a clean VM and see if I can debug there, but not tonight unfortunately.

[jhamrick]

Worked a bit more on this tonight, and just to document my progress:

• The reason the nbextensions tests are failing is because the nbgrader nbextensions are now causing the notebook to fail to load entirely (the browser just hangs trying to load the URL).
• The reason the notebook is hanging seems to still have to do with the asyncio issue.
• I /think/ the problem is that in order to use nbconvert, we need to set the event loop to WindowsSelectorEventLoopPolicy. But tornado 6.1 is now configured to use the default asyncio event loop, and the notebook requires tornado 6.1, so the workaround of setting WindowsSelectorEventLoopPolicy doesn't work and causes the notebook to hang (I still don't fully understand this interaction and why this is happening, but this is my best guess).
• I can move the asyncio workaround so that it only applies when we run nbgrader from the command line, but then the nbconvert functionality run from the formgrader and validate assignment extensions doesn't work again.
• Two possible solutions, which I did not have time to investigate tonight:
  • Require python 3.7 for windows users. This seems suboptimal though and will likely cause a lot of questions and headaches.
  • Maybe we can launch the autograder (and other nbconvert functionality) in a subprocess when we're on windows and running the formgrader, at least for now. But I am not sure if the event loop can be configured per-process or if it's somehow tied to the root/parent process though.

[jhamrick]

Ok, I worked on this for quite a while tonight as well. I attempted to shell out the autograder and validation code into a subprocess so that it uses a different event loop, and this does indeed work, but creates a whole host of other problems---the subprocess do not inherit configs loaded in the parent process, the validator returns a structured dictionary and so I have to save this to a temporary file and the load the results in the parent process, etc. It's getting to the point where this is a very ugly hack and I'm not feeling comfortable with this direction 😞

What I don't get more generally is why this is a problem for nbclient (nbconvert) but not the notebook itself. I get that the code to execute a notebook is separate (https://github.com/jupyter/nbclient/blob/master/nbclient/client.py vs https://github.com/jupyter/notebook/blob/a5ace13e0a5d99f314e3b933b2b079ff0bffbd5a/notebook/services/kernels/handlers.py) so maybe there is some difference there?

I think I'll go comment on the nbclient repo and see if anyone there has any suggestions.

[perllaghu]

Whilst the spinout is turning into an ugly hack..... a revisit at later date could be good: if we can un-couple the autograding from the instructors notebook-server... then it makes multiple markers a more interesting solution (shared/centralised gradebook.db-esqu database, shared grading filestore, etc...

[jhamrick]

Yeah, that's actually more like how the autograder used to work---it used to be a completely standalone process. We made the move to have it be a notebook extension so that it was easier to install, but I agree that choice has made it more complicated and confusing in some ways as well. Though I'll note, you don't necessarily have to couple it with the instructors' notebook, it can be its own separate notebook server (it just requires a notebook server) which you can grant multiple instructors access to via JupyterHub groups.

[minrk]

After looking at how the extensions are organized, I'm not sure this import happens for the server extensions. Try moving this to a step in nbgrader.apps.baseapp.NbGrader.initialize(), then it might be enough.

[orboan]

Hello, is nbgrader already compatible with nbconvert v6? Have you managed to fix this issue?
Downgrading nbconvert to v5 is not an option for me as I need SoS kernel which is only compatible with nbgrader v6.

Thanks for all your effort. I would like to contribute but my knowledge and time are limited.

Thanks again

[poldap]

@orboan we are on the same boat. We keep a separate virtualenv to generate feedback files for now.

@jhamrick can we help somehow? Thanks for all the efforts.

[jhamrick]

Superseded by #1567