-
Notifications
You must be signed in to change notification settings - Fork 586
Add Latex A4 index file #2222
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
base: main
Are you sure you want to change the base?
Add Latex A4 index file #2222
Conversation
for more information, see https://pre-commit.ci
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.
I personally don't think adding a new template is the way to fix this problem when the material difference is the papersize
parameter to \documentclass
. Rather than forking the whole template, could a variable be extracted?
For example, look at the way HTMLExporter
handles the customizable mathjax_url
using _init_resources()
to pass that value along to share/templates/lab/index.html.j2
.
If you instead implemented something similar on LatexExporter
, that could additionally enable an alias like --paper-size A4
or even an --a4
flag, which feels like a more intuitive way to use a different size.
Whilst I agree it isn't the most intuitive api. We can't simply extract just the document class variable since the ((* set charlim = 77 *)) is also dependant on other aspects of the template. For example, this value in my templates is anywhere from 80 to 94 depending on the margins and prompt formatting in the latex, but this cannot be set at latex runtime since this number is used in pandoc. Meaning we'd need to adaptively pull this number from the template (which could be set at multiple points) and change it and put it back into the template. Otherwise we'd mess with 3rd party templates. Maybe we could add a different variable for an if statement and to switch between charlim/document class combos in a single index file. But in my opinion, this really does belong in a different template file from the latex side, since 3rd party templates may or may not wish to support different paper sizes including those beyond letter and a4. This isn't forking the entire template, just the index file. The underlying |
I see what you mean that you're not forking the entire template, but the new file is in Can you say more about My proposed alternativeInstead of creating separate template files, I'm proposing that we handle this in the Traitlets configuration. # In LatexExporter
paper_size = Unicode("letterpaper", help="named LaTeX paper size").tag(config=True)
charlim = Int(help="text wrap width").tag(config=True)
@default("charlim")
def _default_charlim(self):
return { "letterpaper": 80, "a4paper": 77 }.get(self.paper_size, 80)
def _init_resources(self, resources):
resources = super()._init_resources(resources)
resources["paper_size"] = self.paper_size
resources["charlim"] = self.charlim
return resources ((=- In index.tex.j2 -=))
((* set paper_size = paper_size or resources.paper_size or "letterpaper" *))
((*- block docclass -*))
\documentclass[11pt,(((paper_size)))]{article}
((*- endblock docclass -*))
((=- In style_jupyter.tex.j2 -=))
((* set charlim = charlim or resources.charlim or 80 *)) This approach would:
Regarding 3rd party templatesI think this is worth more consideration. Different sizes weren't supported before, so adding this support won't necessarily break existing code, but it does add on to the API that nbconvert needs to maintain moving forward. I searched GitHub for instances of others using this LaTeX template, and most users would either be unaffected by this change, or completely replace the template anyway. Comparing these two options for adding an alternate size API, there are some strengths to the config approach:
And I think this template-based approach has some weaknesses:
Both options are easy to ignore for users who don't want to use them. You mentioned 3rd party templates which may not want to support paper sizes. I don't have any evidence for this, but I suspect the number of users who would welcome the new functionality is greater than the number would explicitly not want it. For those who don't want to allow alternate sizes, it's easy enough to add the following to their custom template to opt out.
On ScalabilityMoving forward, let's say we want to support more paper sizes and landscape orientation as well. Would that look like the following?
The tl;drIn my view, the config route allows us to support more functionality out of the box more easily while accumulating less technical debt. |
Correct, the location of the file is wrong; good catch. I'll fix that (I was doing my testing in a different config). Perhaps you're correct about making the paper size configurable as a string traitlet. I'd probably not make charlim a traitlet though. I'd probably check the papersize in the template and set the value there based on the papersize value, and keep all the logic for the various supported papersizes in the index file. Charlim really should be set by the template. Setting this in the index file is better so we don't have to adjust the other cell styles in the future if those are ever updated to use charlim. We'd have to document which papersizes the default template supports and that different templates may support different papersizes. Actually, what if a template wants to support A4 and A3 only? Then the default letterpaper makes zero sense, and you can't exactly ignore it. In this case, I think we need to adjust the template's As far as scalability goes, I'm not so concerned. Anything outside of letter/A4 really is outside the scope of 1st party support. This is just about supporting 1 imperial paper size and 1 metric by default. Charlim is only used for text wrapping for plain text outputs. It ensures that the text doesn't go off the size of the page or get auto-wrapped by fancyvrb with a bunch of red markups like the code fancyvrb is designed to do. I hardcoded it back in #992 when the jupyer cell style was introduced by me, as it worked for the page width. |
@SylvainCorlay If I recall correctly, you were heavily involved in the transition to our modern templating system that added the conf.json. Thoughts on using it+traitlets for this purpose vs separate index files? Basically all my work on NBconvert was prior to that change. |
Ope. I didn't think of that. You're right that hard-coded traitlet defaults don't make sense inside the exporter (except in the case of backwards compatibility). Default values instead belong in conf.json where the template designer has control. However, if the template supports more than one size, the traitlets layer is where the end user would select which one they want, so the trait should exist, just not with a hard-coded default. So if I'm understanding everything correctly, the way forward would be to add:
Reading conf.json
>>> LatexExporter()._get_conf()
{
'base_template': 'base',
'mimetypes': {'text/latex': True, 'text/tex': True, 'application/pdf': True}
} On Charlim (forget I said anything 😅)
We're thinking about this value in a different way. I've been thinking about charlim like vim's
As you said, the use of |
Fixes #392
The user can force A4 with the
--template-file index_A4
flag on the command line orc.LatexExporter.template_file = 'index_A4.tex.j2'
from the appropriate config file. A4 is narrower, so I had to adjust the textwrapping limit down from the default of 80 we use for letter.I also intend to update my templates over at t-makaro/nb_pdf_template as well.
There is one side effect that I noticed. If a user sets the index file to
index_A4.tex.j2
AND a template that otherwise doesn't have this file (for example using a template from my repo above), it will actually still convert with A4 paper, but the charlim might not be set correctly (too big with my template as is which doesn't allow this to be overridden, and it's too small if my template lets it be overridden). Granted this doesn't prevent me from adding my own index_A4.tex.j2 template, and working exactly as expected, but it's just an oddity that it doesn't fail. I presume this is because the folder search order is falling back as needed it ends up with a bit of a hybrid between the templates. Realistically, this is really user error for trying to use a feature that isn't supported by the template that they are trying to use.