Fix nbconvert handler #2981
Fix nbconvert handler #2981
Conversation
|
I can't restart the Appveyor build… it looks like that error might just go away since i just verified that the conda-forge package does exist… But for some reason I can't log in to appveyor. |
|
Can someone please add me to the list of those with permissions to restart appveyor builds? I don't understand why it doesn't just use github permissions, but it seems to not do that. |
|
@Carreau Do you have those powers? |
|
@gnestor I really don't think this should wait until 5.3… instead aiming at 5.2.1 or are you using 5.3 to also organise patch releases for 5.2.x? |
|
We don't have a milestone for 5.2.1 so I marked it as 5.3. I'll create one now and shoot for releasing 5.2.1 tomorrow. |
notebook/nbconvert/handlers.py
Outdated
| self.set_header('Last-Modified', model['last_modified']) | ||
|
|
||
| # create resources dictionary | ||
| mod_date = model['last_modified'].strftime(text.date_format) | ||
| nb_title = nb.metadata.get("title","") or os.path.splitext(name)[0] |
There was a problem hiding this comment.
Not convinced about the title from notebook metadata taking precedence over the filename used to identify it. The notebook metadata is already available in the template, it doesn't need to be added to resources.
There was a problem hiding this comment.
Fine, this was a fix that I had added on the nbconvert side for the LaTeX interpreter and was just trying to keep it consistent here. But I'm not wedded to it.
|
The 5.2.1 branch is ready for release so we're just waiting on this PR 👍 |
|
fixed! |
|
I tested and export to PDF is working 👍 @mpacer Shall I merge? |
|
@gnestor 🚢 |
|
@meeseeksdev backport to 5.2.1 |
|
Oops, something went wrong applying the patch... Please have a look at my logs. |
* get the directory for external resources to pass in as path to nbconvert * build up resources dict elements separately, combine before passing to from_notebook_node * Use filename (not title) for the name used as the title in html export
|
I backported this manually to 5.2.1 👍 |
|
Apologies! didn't see you ask me if it could be merged — I had intended it to be done, yes. Any thoughts as to why the meseeksdev backport didn't work? |
|
No prob! No I don't have access to the meseeksdev logs but there must've been a merge conflict |
ghost
mentioned this pull request
closes #2974,
Other small improvements while I was doing this:
Building the resources dict separately from invocation is more readable.
resources['metadata']['name']now will use the notebook level metadata.title value as passed through to name (if present) and fall bad to stripping the extension using os.path.splitext. This notion ofnameis only used in the html and slides exporters to create a<title/>tag.