rstudio / rticles

LaTeX Journal Article Templates for R Markdown
https://pkgs.rstudio.com/rticles/
1.46k stars 516 forks source link

Fix issue with mnras format #403

Closed cderv closed 3 years ago

cderv commented 3 years ago

Seems like there is an issue with the mnras rticles format

https://github.com/rstudio/rticles/runs/2637643070?check_suite_focus=true#step:15:229

cderv commented 3 years ago

Issue is

! Incomplete ifx; all text was ignored after line 3.
<inserted text> 
                fi 
l.112 ...\familydefault\seriesdefault\shapedefault
cderv commented 3 years ago

I am not sure this comes from our template. mnras files are from CTAN : https://www.ctan.org/tex-archive/macros/latex/contrib/mnras

@oleskiewicz pinging you in case you know better as you contributed the template in the first place I believe.

I am not sure to know where this error comes from in the tex file.

oleskiewicz commented 3 years ago

When I added the template, I aimed to faithfully translate the example from: http://mirrors.ibiblio.org/CTAN/macros/latex/contrib/mnras/, I'm not aware of any changes to the template since.

I took a look, but I can't figure out what the error is, or why it started failing now.

I'll try reproducing it locally in a fresh environment and sending a PR if I manage to find a fix.

oleskiewicz commented 3 years ago

I managed to build a new article with

rmarkdown::draft("article.Rmd", template = "mnras", package = "rticles")
rmarkdown::render("./article/article.Rmd")

but I had to add http://mirrors.ibiblio.org/CTAN/macros/latex/contrib/mnras/mnras.cls file to ./article/ directory, and manually delete vadjust environment (this I believe was solved by #383. So I think we can just add mnras.cls file to the skeleton directory and that should fix it!

cderv commented 3 years ago

Thanks @oleskiewicz !

mnras.cls is not included into rticles because it should be available on CTAN (which is where you pointed to) and installed by tinytex when the Rmd is converted.

> tinytex::parse_packages(files = "mnras.cls")
tlmgr search --file --global "/mnras.cls"
[1] "mnras"

I'll look into this.

cderv commented 3 years ago

@oleskiewicz I tested a wild thoughts: would the issue have fixed itself... So I triggered a recheck of the actions to see if there would still be an error and it seems not. See #408 -> every checks are green ✔️ !

It means that it could have been an issue with the mnras package not being properly fetch the first time.

This now works for me on my side

withr::with_tempdir({
  rmarkdown::draft("article.Rmd", template = "mnras", package = "rticles", edit = FALSE)
  rmarkdown::render("./article/article.Rmd")
})

So it seems there is no more issues with it. I would prefer not to include the cls file in the package as it is available on CTAN (to keep the size small and avoid update issues in the future)

Did you reproduce the error in the first place when you tried 2 days ago ?

Sorry to have pinged you with this as it seems to has been solved now. 😓

oleskiewicz commented 3 years ago

If tinytex includes the mnras class file, then that's even better! Indeed, no need to include the whole thing twice.

I think in this case it's ok to revert the change & close the issue.

Sorry to have pinged you with this as it seems to has been solved now. 😓

No trouble at all.

cderv commented 3 years ago

If tinytex includes the mnras class file, then that's even better! Indeed, no need to include the whole thing twice.

tinytex won't include the file. It will download it from CTAN if missing.

I think this is fixed somehow so I'll close this and we'll reopen if it happen again.

Thanks !

github-actions[bot] commented 2 years ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.