Skip to content
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

PDF: Enable whole of project book style PDF #299

Merged
merged 107 commits into from
Nov 18, 2019
Merged

PDF: Enable whole of project book style PDF #299

merged 107 commits into from
Nov 18, 2019

Conversation

mmcky
Copy link
Contributor

@mmcky mmcky commented Oct 23, 2019

This PR enables a whole of project book style PDF

@mmcky
Copy link
Contributor Author

mmcky commented Oct 23, 2019

the index_toc document should be passed in as a configuration variable to indicate which document contains the book toc for pdf

@@ -68,6 +68,11 @@ def __init__(self, builder, document):
# Slideshow option
self.metadata_slide = False #False is the value by default for all the notebooks
self.slide = "slide" #value by default

## pdf book options
self.in_index_toc = False
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's name this self.in_pdf_index

@@ -78,6 +83,13 @@ def visit_document(self, node):
"""
JupyterCodeTranslator.visit_document(self, node)

## if the source file parsed is index_toc and target is pdf
if "index_toc" in self.source_file_name and self.jupyter_target_pdf:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should replace index_toc with a variable from conf.py indicating which document contains the toc for book style PDF

@mmcky
Copy link
Contributor Author

mmcky commented Nov 12, 2019

thanks @AakashGfude the testing is looking good -- I had waited for some changes in the python lecture repository such as a change to index but the extension handled these changes nicely.

I just need to fix CI and then I will merge these changes into master and release 0.5.2 with book support.


for path in [self.pdfdir, self.texdir]:
ensuredir(path)

self.pdf_exporter = PDFExporter()
self.tex_exporter = LatexExporter()
self.index_book = builder.config['jupyter_pdf_book_index']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AakashGfude let's leave this for now - but it is confusing that this configuration has a different name

@mmcky
Copy link
Contributor Author

mmcky commented Nov 12, 2019

  • test for book is not setup but will merge this once CI is green and add that test in later.

@mmcky mmcky added in-review and removed in-work labels Nov 12, 2019
fl_tex = self.texbookdir + "/" + self.index_book + ".tex"
filename = self.index_book

## checking if an explicit output filename is specified in the config file
if "jupyter_pdf_book_name" in builder.config and builder.config["jupyter_pdf_book_name"]:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AakashGfude I suspect this needs to check jupyter_pdf_book as the first condition above will always be met even if it is None (i.e. as the default value)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AakashGfude can you check this condition?

@mmcky mmcky added ready and removed in-review labels Nov 18, 2019
@mmcky
Copy link
Contributor Author

mmcky commented Nov 18, 2019

fixes #277

@mmcky
Copy link
Contributor Author

mmcky commented Nov 18, 2019

the need to update docs has been captured by #302

@mmcky mmcky merged commit 21591a3 into master Nov 18, 2019
@mmcky mmcky deleted the making-book branch November 18, 2019 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants