NBConvert 6.0 Planning #1045
Comments
I agree. It's weird that a conversion utility deals with execution, but we should keep the
Yes please. Note: Python 2 notebooks will still be able to be converted. Nbconvert itself will just need python 3 to run.
This will have a more clear path forward after I do issue triaging.
I'd be willing to help audit some of the configuration options. We could definitely do with some pruning. I'd also like to re-write some of the docs to make the python API and configuration options more clear. This would also make the repo more contributor friendly.
As an example of bugs: #774. That said. Traitlets and python based config files really do allow for a lot awesome config. Refactoring and lightening would be appreciated. I feel like a big problem with traitlets is the documentation. I had to look through the code to find Should we do 5.6 release for some of the fixes that aren't API breaking? It would also be nice to have milestones for "must be in >6.0" and "could be released before 6.0". It would help with triaging issues. |
|
Note on traitlets. It actually helped me get into contributing to nbconvert when I was making #889 once I realised that all I had to do was add 1 line to make something easily configurable. I really think the limitation is in documentation more than anything. |
|
Don't get me wrong that the ability to configure is really nice -- it's the debugging it when it goes wrong (which I just spent a good 5 hours doing so last week) is where I find myself questioning if we need something so complicated. |
|
For the tagging I say we add anything that should be > 6.0 as Milestone 6.0 for now. Anything not milestoned could be before or after. |
|
Can we consider making to be the command for notebook -> notebook operations. This way if people just want to apply preprocessor operations without changing to a different format it doesn't need an explicit |
I'm +1 for this |
👍
I certainly think traitlets are an attractive nuisance. They're a whole pile of complexity which is, I believe, largely unnecessary for this kind of application (I believe traits were originally designed to support making GUIs quickly). And they tie your config to class names, making refactoring a pain. The one thing in their favour is that it's really easy to add a new config option. But, I'd argue, it's kind of too easy. You wind up making everything configurable because why wouldn't you add OK, enough ranting.
Maybe with a warning, in particular if there are no preprocessors configured? I suspect there are a lot of people who are used to not needing If I can suggest one further idea: drop postprocessors as a general category? There's only one (serve), and that only really makes sense for one specific format (slides). Maybe there should be a separate little tool that converts a notebook to reveal slides and serves them? |
I think this could make sense for several of the generic preprocessors which manipulate the notebook data, e.g. removing cells, clearing metadata. Like with execution, there's no real reason that they need to depend on the conversion machinery. Maybe they should live together in a package called something like |
I'd be down for dropping the
We were thinking the execute preprocessor should have it's own isolated package because papermill, nbconvert, and possibly a future async client library would all make use of just that dependency. Currently papermill pulls a lot of extra dependencies it doesn't need to get nbconvert installed. Would other preprocessors you've read have other direct importers that wouldn't want the rest of nbconvert? I'd be inclined to leave processors that are only used by nbconvert in the library, as we already have a lot of problems with users having mismatched dependency versions that don't work together. Would like to iterate on the ideas more here. |
nbstripout is doing something a lot like the This is really going one step further with @t-makaro's idea to make |
|
Brainstorming: would it make sense to specify a kind of in-memory pipeline, something like this: |
|
I think chaining processors would be a good move. It would make the ordering of conversions clear and allow for one call to do all conversions. It also makes pre vs post processor not matter anymore. |
|
I would be in favor of tagging alpha releases before we check all the boxes here. As soon as we have it, we will start targetting the alpha in a development branch of Voilà and other places where we consume the new template system. |
|
I can look this weekend at any remaining python 3-only items and kick off an alpha release to that effort. @takluyver anything you're aware of that we should close before doing so? |
|
I haven't been following closely, but I think you can make alpha releases whenever you think it's useful. Just be aware that anyone running |
|
@SylvainCorlay pre-release |
|
Unfortunately notebook tests have picked up the pre-release, and are failing with an error about not finding templates: |
|
I'm not sure that's unfortunate or not. Is the build purposefully pulling
any (not released) dependency versions?
It does tells us where we need to prep fixes for 6.0 branch. I am guessing
that code needs to be 5.x/6.0 compatible.
…On Sun, 24 Nov 2019, 1:58 pm Thomas Kluyver, ***@***.***> wrote:
Unfortunately notebook tests have picked up the pre-release, and are
failing with an error about not finding templates:
Traceback (most recent call last):
File "/home/travis/build/jupyter/notebook/notebook/nbconvert/handlers.py", line 132, in get
resources=resource_dict
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/nbconvert/exporters/html.py", line 99, in from_notebook_node
self.register_filter('highlight_code', highlight_code)
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/nbconvert/exporters/templateexporter.py", line 418, in register_filter
return self._register_filter(self.environment, name, jinja_filter)
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/nbconvert/exporters/templateexporter.py", line 154, in environment
self._environment_cached = self._create_environment()
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/nbconvert/exporters/templateexporter.py", line 436, in _create_environment
paths = self.get_template_paths()
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/nbconvert/exporters/templateexporter.py", line 465, in get_template_paths
template_names = self.get_template_names()
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/nbconvert/exporters/templateexporter.py", line 512, in get_template_names
raise ValueError('No template sub-directory with name %r found in the following paths:\n\t%s' % (template_name, paths))
ValueError: No template sub-directory with name 'classic' found in the following paths:
/tmp/tmpjcjzbt1f/data
/tmp/tmpjcjzbt1f/env/share/jupyter
/tmp/tmpjcjzbt1f/share/jupyter
https://travis-ci.org/jupyter/notebook/jobs/616385738
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1045?email_source=notifications&email_token=AAOHBVA5EQQ77VDU7LPECATQVL2H5A5CNFSM4HWOEWPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFAV5KQ#issuecomment-557932202>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOHBVFKAQKVLLAJDE5LLTTQVL2H5ANCNFSM4HWOEWPA>
.
|
|
It deliberately uses I'm guessing the issue is that the template files are now found using the 'jupyter path' search path, but the notebook tests monkeypatch this to isolate themselves from the user's local setup. Certainly the notebook code and test suite needs to remain compatible with nbconvert 5.x at least until nbconvert 6 is released (and really for a while afterwards too). |
|
The notebook issue only seemed to be test run issue: jupyter/notebook#5092 jupyter/notebook#5086. I'm now working on voila and many of the templates, so far so good. |
|
Alright, that works |
|
FYI, I pushed another quiet alpha (6.0.0a4), so that we can start testing the latest changes in |
|
Sounds good, I think some rapid alpha releases to support testing the last changes is a good idea. Btw in helping with #1292 I think I got most of the "backwards compatability / warnings for old template patterns" done if you wanted to take a look later. |
|
I am thinking of kicking off a beta release and setting an explicit date for 6.0 official release. I'd prefer to do the full release in a week or two (at most). The only things I have that I am queuing to complete are:
There's a number of bugs still left in https://github.com/jupyter/nbconvert/issues?q=is%3Aopen+is%3Aissue+milestone%3A6.0 but nothing I'd consider blocking a release. I am going to post to discord about seeing if anyone that's not a common contributor is open to helping wrap those up before a release goes out. What are folks thoughts on kicking off a beta release now and a full release next ~weekend? Is there anything else that needs to be actively addressed @t-makaro @maartenbreddels @SylvainCorlay ? |
|
Oh and #582 perhaps if we want to take an action around PostProcessors or not. |
|
I kicked off a 6.0.0b7 release for the beta and am going to announce it on the discourse so people are aware / ask for interested parties to contribute any of the open bug fixes they want in before the final release. |
why not |
|
I think it is really weird to release the first beta with |
|
It was the natural progression of pre-release numbering from a build number perspective after a6. It also showed that we had several prior builds, and the number doesn't really matter as it's not semver anyway. I'd prefer we just leave it to avoid confusing people who have already installed the latest pre-release because pip tooling doesn't do great with installing when you go backwards and not forwards with release progression alphanumerically. |
|
OK, sounds good. |
|
Nbconvert 6.0.0rc0 is out with the latest changelog. I sent another round of emails to downstream maintainers about it. |
If you don't have time to tackle this, I think it would be great to table it to the next release. We are blocking on nbconvert 6.0 for quite some time, and the more we wait, the more difficult it becomes...
I opened a PR to the readme to add the comment |
|
@SylvainCorlay That's fair, I was just going to take some time today to see if I could get it resolved. I agree and would like to release 6.0 this week. Give me a couple hours and I should have that PR updated, but I agree that we don't need to block on it. |
|
Great! let's aim for tomorrow (Tuesday)! |
|
Excited for 6.0, thanks for working on this release!
It seems like this is only an issue with installing nbconvert zip or tar.gz files from GitHub, not directly from a commit, so likely won't be a problem for the release. Sorry for the noise. |
|
Np, thanks for reporting. I'll follow up on that oddity in #1358 but I agree I'm not hitting a blocking issue for release. |
|
@SylvainCorlay Since we're in fairly different timezones how do you want to coordinate the release itself? I did a PR using the release automation tooling to capture the remaining authors for this release: #1363 I'm happy with either of us running the release after that merges. |
|
What about #1347? |
|
Ahh we should check that and see if it's an issue or not (I won't have any time tomorrow to do that) |
|
There is still an error in the example notebook mentioned in #1339 (see also #1292 (comment)). This doesn't yet show up as a CI failure (see #1350), but I think the notebook itself (or whatever is causing the error) should be fixed before 6.0 because the failure will be visible in the docs, where the example output is much too long: https://nbconvert.readthedocs.io/en/latest/nbconvert_library.html#Example |
|
@MSeal @maartenbreddels I fixed the last issue reported by @mgeier. There is the question of #1347, which is about supporting the cwd as a template path. I am not sure we should do this though. |
|
OK, I added a comment about #1347. If nobody objects by then, I will tag 6.0.0 tomorrow morning (CET)! |
|
I think we're good for a release, thanks for offering to handle shipping @SylvainCorlay. If minor things come up after let's just be more proactive with patch releases with fixes than we were with 5.x. |
|
Nbconvert 6.0 is out! 🎉 The conda-forge PR should follow shortly. Closing this issue! |
|
I can post on discourse later today or tomorrow about the event. Awesome work everyone and thanks for getting 6.0 across the finish line |
In light of several conversations around larger changes, I wanted to start the thread on 6.0 targets and outline / discuss what changes should or shouldn't be in the next major release. We should make issues for each of the following that we want addressed. Any of the topics below can be questioned and challenged or pushed until later releases. Champions for each one would be appreciated to help move key subjects forward. I'll keep this top ticket updated with links and additional requested items from the thread below.
@mpacer first brought up the idea, I think it would greatly help the separation of concerns and let nbconvert focus on conversion rather than execution. Other projects (i.g. papermill) that just want execution ability could import this new module rather than the much larger dependency chain of nbconvert.
Remove, or at a minimum not support, python 2 moving forward. Seems straight forward enough for the major version upgrade.
First part for adding additional paths will be in make a default global location for custom user templates #1028 . It's intended to make templates a directory with resources and multiple asset files rather than a single template file with the jump to 6.0.
This is a tall order, and we've been trying to get improvements made for the major latex / pdf issues, but there's a lot of these and I'm not sure we'll have them closed for the last 5.x release. Will probably require some significant improvements to how templates are loaded, executed, and tested to get improvements to more production quality levels.
- [ ] Deprecate / remove unnecessary or complicated configuration optionsWe have a lot of configuration options, some of them are old or could be unnecessary now. I'd like to prune options and figure out how templating improvements could help where flags were used as crutches in the past. Not sure what the full scope could be here for this.
- [ ] Consider lightening or removing traitletsNot going to be a popular topic, but traitlets causes a lot of support burden and debugging issues. It's also been cited as the part that's kept many strong developers I know from contributing because it's complicated and difficult to understand how it operates without a lot of digging. I don't expect this topic to make it in for 6.0 but I thought I might at least spark a conversation on the topic to see how we could make the repo more contributor friendly.
There's probably some more I'm not thinking of atm. I'll add items from the thread that seem relevant.
The text was updated successfully, but these errors were encountered: