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

Consider switching from weasyprint to another pdf converter #170

Closed
vincerubinetti opened this issue Jan 28, 2019 · 7 comments · Fixed by #210
Closed

Consider switching from weasyprint to another pdf converter #170

vincerubinetti opened this issue Jan 28, 2019 · 7 comments · Fixed by #210

Comments

@vincerubinetti
Copy link
Collaborator

I noticed that manubot used to use wkhtmltopdf but switched to weasyprint due to the former lacking svg capabilities.

I've just run into a deficiency in weasyprint, in that it doesn't support the CSS calc() value. We don't absolutely need to use the calc() value, at least right now. Conceivably in the future, it could be required to achieve a certain look/layout in a "clean" way compliant with modern best practices.

I'm wary that weasyprint reimplements its own new rendering engine, instead of using a well established standard like webkit or gecko. Even if weasy somehow does basic rendering as good as or better than webkit/gecko (projects that have hundreds of contributors), I think it will necessarily miss some of the more advanced functionality of CSS that we might want/need to use. This calc() value for example isn't exactly a cutting edge CSS feature; it came into being around 2011.

Again, it's a small problem, but it may be worth it to research tools like athena and prince, and see if they're more fully featured (preferably based on webkit) than weasyprint, and would work with pandoc and our build process.

@vincerubinetti vincerubinetti changed the title Consider switching off weasyprint Consider switching from weasyprint to another pdf converter Jan 28, 2019
@dhimmel
Copy link
Member

dhimmel commented Jan 28, 2019

Sure happy to consider open source alternatives to weasyprint.

I can't speak regarding the capabilities of athena or prince, but I've been pretty satisfied with the functionality of weasyprint and exceptionally happy with the support. Whenever we report issues, we've responses quickly... even when their not really weasyprint issues.

Not to say we shouldn't evaluate alternatives if there are important missing features. It probably makes sense to revisit this issue more actively if we decide to switch to a manubot Docker image that could more readily include dependencies that are not already part of conda-forge.

@dhimmel
Copy link
Member

dhimmel commented Feb 12, 2019

Random links to look into more:

@dhimmel
Copy link
Member

dhimmel commented Feb 12, 2019

I tried athenapdf via it's Docker to create a PDF of the bitcoin-whitepaper:

docker run --rm \
  --volume=`pwd`:/converted/ \
  arachnysdocker/athenapdf:2.16.0 \
  athenapdf \
  --delay=2000 \
  https://git.dhimmel.com/bitcoin-whitepaper/v/ebfce2b530b141764275093eee8f424b18144420/

The output was 434d484a41538079696897478e6ff7d532f32871.pdf. It looks great! I had to increase the delay from the default of 200 milliseconds, such that the MathJax equations would finish rendering. Really happy that it was able to convert the equations.

@slochower
Copy link
Collaborator

Looks like support for page numbers is still nontrivial. https://github.com/arachnys/athenapdf/issues?utf8=%E2%9C%93&q=numbers

@dhimmel
Copy link
Member

dhimmel commented Apr 17, 2019

We have had good success with Athena for the Meta Review. @slochower also reports success with Athena in greenelab/meta-review#205 (comment).

In greenelab/meta-review@cffcfea, we default to using Athena when available, but fall back to the current WeasyPrint command when Docker is not available. This is one possible implementation.

Another possibility is for us to only build PDFs when Docker is installed and BUILD_PDF!=false. This would allow us to simplify our environment.yml and resolve some frustrating conda dependency incompatibilities we had in #174. PDFs will continue to be built on Travis. This option also prevents two PDFs for the same HTML existing which could get a bit confusing.

Is the ability to create PDFs locally without Docker sufficient to justify the increased support burden of keeping WeasyPrint?

@agitter
Copy link
Member

agitter commented Apr 17, 2019

I've encountered a situation where I wanted to build a local PDF on a machine that did not have Docker. Could we adopt the meta-review approach for now and wait until the environment management becomes too burdensome to drop WeasyPrint?

How would two PDFs be built from the same HTML? If a user builds once on a machine with Docker and again on a machine without?

@dhimmel
Copy link
Member

dhimmel commented Apr 17, 2019

How would two PDFs be built from the same HTML? If a user builds once on a machine with Docker and again on a machine without?

Yes, it's a minor issue, but the local PDF may differ from the travis PDF, which could cause some confusion. For example, if we continue to get bug reports about features not working locally due to WeasyPrint.

Could we adopt the meta-review approach for now and wait until the environment management becomes too burdensome to drop WeasyPrint?

Okay.

dhimmel added a commit to dhimmel/manubot-rootstock that referenced this issue Apr 17, 2019
Closes manubot#170

More information about Athena is available at
https://github.com/arachnys/athenapdf

Apply feature from meta-review commit:
greenelab/meta-review@cffcfea

This commit was created with the command:
curl https://github.com/greenelab/meta-review/commit/cffcfeaea0b6859051d16ab1e263d8bcf62a24b1.patch | git apply
dhimmel added a commit that referenced this issue Apr 24, 2019
Merges #210
Closes #170

More information about Athena is available at
https://github.com/arachnys/athenapdf

Based on meta-review commit:
greenelab/meta-review@cffcfea
dhimmel added a commit that referenced this issue Apr 24, 2019
This build is based on
0e29315.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/manubot/rootstock/builds/109471160
https://travis-ci.com/manubot/rootstock/jobs/195308073

[ci skip]

The full commit message that triggered this build is copied below:

Use Athena to build PDF, if Docker installed

Merges #210
Closes #170

More information about Athena is available at
https://github.com/arachnys/athenapdf

Based on meta-review commit:
greenelab/meta-review@cffcfea
dhimmel added a commit that referenced this issue Apr 24, 2019
This build is based on
0e29315.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/manubot/rootstock/builds/109471160
https://travis-ci.com/manubot/rootstock/jobs/195308073

[ci skip]

The full commit message that triggered this build is copied below:

Use Athena to build PDF, if Docker installed

Merges #210
Closes #170

More information about Athena is available at
https://github.com/arachnys/athenapdf

Based on meta-review commit:
greenelab/meta-review@cffcfea
slochower added a commit to slochower/smirnoff-host-guest-manuscript that referenced this issue May 15, 2019
* webpage.py: note to ignore error in versions checkout

Merges manubot/rootstock#211
Closes manubot/rootstock#183

* Use Athena to build PDF, if Docker installed

Merges manubot/rootstock#210
Closes manubot/rootstock#170

More information about Athena is available at
https://github.com/arachnys/athenapdf

Based on meta-review commit:
greenelab/meta-review@cffcfea

* Add meta review link to readme

Merges manubot/rootstock#217
Closes manubot/rootstock#216

* Increase shared memory of athenapdf container

Merges manubot/rootstock#220
Closes arachnys/athenapdf#195

Increase --shm-size from the default value of 64m to 1g when
running the athenapdf Docker image. Resolves athena issue:
`The renderer process has crashed`

Switch to using equal sign to separate Docker arguments
and values for consistency.
https://stackoverflow.com/q/50319060/4651668

* CSL author macro: substitute editor/venue

Merges manubot/rootstock#219

Ensures there is a newline between the title and venue,
when authors are missing.
Closes manubot/rootstock#218
Supersedes manubot/rootstock#158

Substitutes editors for authors when authors are missing.
Editors are labeled like "Sönke Bartling, Sascha Friesike (editors)".
slochower added a commit to slochower/smirnoff-host-guest-manuscript that referenced this issue May 15, 2019
This build is based on
d62afc4.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/slochower/smirnoff-host-guest-manuscript/builds/532938787
https://travis-ci.com/slochower/smirnoff-host-guest-manuscript/jobs/532938788

[ci skip]

The full commit message that triggered this build is copied below:

Rootstock 2019 05 15 (#36)

* webpage.py: note to ignore error in versions checkout

Merges manubot/rootstock#211
Closes manubot/rootstock#183

* Use Athena to build PDF, if Docker installed

Merges manubot/rootstock#210
Closes manubot/rootstock#170

More information about Athena is available at
https://github.com/arachnys/athenapdf

Based on meta-review commit:
greenelab/meta-review@cffcfea

* Add meta review link to readme

Merges manubot/rootstock#217
Closes manubot/rootstock#216

* Increase shared memory of athenapdf container

Merges manubot/rootstock#220
Closes arachnys/athenapdf#195

Increase --shm-size from the default value of 64m to 1g when
running the athenapdf Docker image. Resolves athena issue:
`The renderer process has crashed`

Switch to using equal sign to separate Docker arguments
and values for consistency.
https://stackoverflow.com/q/50319060/4651668

* CSL author macro: substitute editor/venue

Merges manubot/rootstock#219

Ensures there is a newline between the title and venue,
when authors are missing.
Closes manubot/rootstock#218
Supersedes manubot/rootstock#158

Substitutes editors for authors when authors are missing.
Editors are labeled like "Sönke Bartling, Sascha Friesike (editors)".
slochower added a commit to slochower/smirnoff-host-guest-manuscript that referenced this issue May 15, 2019
This build is based on
d62afc4.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/slochower/smirnoff-host-guest-manuscript/builds/532938787
https://travis-ci.com/slochower/smirnoff-host-guest-manuscript/jobs/532938788

[ci skip]

The full commit message that triggered this build is copied below:

Rootstock 2019 05 15 (#36)

* webpage.py: note to ignore error in versions checkout

Merges manubot/rootstock#211
Closes manubot/rootstock#183

* Use Athena to build PDF, if Docker installed

Merges manubot/rootstock#210
Closes manubot/rootstock#170

More information about Athena is available at
https://github.com/arachnys/athenapdf

Based on meta-review commit:
greenelab/meta-review@cffcfea

* Add meta review link to readme

Merges manubot/rootstock#217
Closes manubot/rootstock#216

* Increase shared memory of athenapdf container

Merges manubot/rootstock#220
Closes arachnys/athenapdf#195

Increase --shm-size from the default value of 64m to 1g when
running the athenapdf Docker image. Resolves athena issue:
`The renderer process has crashed`

Switch to using equal sign to separate Docker arguments
and values for consistency.
https://stackoverflow.com/q/50319060/4651668

* CSL author macro: substitute editor/venue

Merges manubot/rootstock#219

Ensures there is a newline between the title and venue,
when authors are missing.
Closes manubot/rootstock#218
Supersedes manubot/rootstock#158

Substitutes editors for authors when authors are missing.
Editors are labeled like "Sönke Bartling, Sascha Friesike (editors)".
adebali pushed a commit to CompGenomeLab/lemur-manuscript-archive that referenced this issue Mar 4, 2020
ploegieku added a commit to ploegieku/2023-functional-homology-paper that referenced this issue Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants