-
Notifications
You must be signed in to change notification settings - Fork 567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a notebook cell style to latex conversion #992
Conversation
This is a subset of my work from https://github.com/t-makaro/nb_pdf_template. Also, in the early days of developing this, I found a bug that I couldn't name this file |
CC @mgeier |
Thanks @t-makaro, this looks very nice! If somebody wants to try this, I think that's how it is supposed to be used:
IMHO this should be merged into the default template, it looks just so much better than the current situation! I currently don't have access to a LaTeX installation, so I can't play with the details. According to your screenshot, the only thing I would change is the formatting of the prompt. This is how I've done it in @jfbu helped me a lot with this, probably he has some ideas for this case, too? |
I am getting a lot of latex errors using this new template when passing the results to xelatex They start with:
Attached is a dummy example I made to test it out (extension changed to .txt to make it attachable). The default template successfully converts to latex and then to pdf with a Is there something I am missing or an issue in the template code? |
Sorry, I've been away from my computer. @MSeal This template is meant to be inherited from by setting the |
I ending up having to pass slightly different prompt spacing depending on whether the prompt was for input, text output and other output (images/math etc.). I've now made this style the default, so this closes #323. Assuming tests pass, this is ready to merge. |
I had to adjust the regex in the test for the new template, and I added a new test to still test the |
It seems that the reason that only some versions of python have failing tests is due to using an old version of texlive. Python 3.7 is using a version of texlive from 2015 whereas python 3.6 and below is using a version of texlive from 2013 that does have appear to have the I'm not sure where to go from here. |
I think it is reasonable to assume that users have a more up-to-date version of the |
I think the python 3.7 environment is getting the slightly newer version of tex live since it is using: |
I've now tried it on one of my own notebooks and it looks indeed nicer than previously. But, if I may say so myself, the PDF output of A few observations:
But all those are minor things, I think this is already a great improvement! |
@mgeier Do you think it's at a quality level for the release this week? I also tested it out and it seems like a strict improvement on what's there now. I think I'm a soft +1 to merge before the release and improve afterwardswith the incremental improvements over time after the bigger release. |
Addressing @mgeier's points:
|
To be perfectly blunt, the PDF output has never really looked very good. And I agree, this PR is definitely an improvement. A large one, in fact. If it were up to me, I would include it as-is in the upcoming release. |
@@ -1,10 +1,10 @@ | |||
|
|||
% Default to the notebook output style | |||
((=- Default to the notebook output style -=)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thought is that might be too greedy though with the -
since that consumes whitespace that would previously have been added by the set cell_style
. I think in this case it makes sense, but we should be careful whenever adding or subtracting -
to our delimiters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latex comment has been annoying me for a long time. There are a few other places that contribute to an unnecessary amount of whitespace at the top of the generated latex file that also annoy me when working with the latex file (not that it really matters), but I figured that I would start with this one while I was in that file.
I believe that nbconvert used to have a
style_notebook.tplx
file that used a cell style similar to that of the actual notebook, but it was removed (I can't find when) because it used a lot of vspaces and hspaces, so it had difficulty with alignment. Over the past year and a half, I've written an entirely new implementation that is much, much simpler. Alignment is almost all done by latex, and there are no custom latex drawing commands (previously used to allow boxes to split across pages).Additionally, this new cell style has text wrapping, so that code/output no longer falls off the page. Fixes #323. Code cells even have a nice visual indicator to show that the wrapping is not syntactic. A little bit of magic was required to get verbatim environments to text wrap, but it works.
Ideally, I think this should be the default, but maybe we should leave that until 6.0?