Serenade600 commented 1 year ago

Bug description

If I added the following code

---
format:
  elsevier-pdf:
    fig-pos: 'H'
---

to position all the figures in document, in the output article.pdf file, only figure using format ![]() is positioned as desired. Figure having subfigures using following code is not affected.

::: {#asdf layout-ncol=2}
![]()

![]()
:::

I tried to add fig.pos='H' like

::: {#asdf fig.pos='H' layout-ncol=2}
![]()

![]()
:::

. Still didn't work.

I also checked the output article.tex file. It seems that single figure is converted to \begin{figure}[H] while the side-by-side figure is always \begin{figure}.

I'm on Windows 10. The output of quarto check is:

[>] Checking versions of quarto binary dependencies...
      Pandoc version 3.1.1: OK
      Dart Sass version 1.55.0: OK
[>] Checking versions of quarto dependencies......OK
[>] Checking Quarto installation......OK
      Version: 1.3.336
      Path: E:\quarto\anzhuang\bin
      CodePage: 936

[>] Checking basic markdown render....OK

[>] Checking Python 3 installation....OK
      Version: 3.7.4
      Path: E:/python/anzhuang/python.exe
      Jupyter: (None)

      Jupyter is not available in this Python installation.
      Install with py -m pip install jupyter

[>] Checking R installation...........(None)

      Unable to locate an installed version of R.
      Install R from https://cloud.r-project.org/

At current situation, I could directly add [H] in the output article.tex file as a workaround. But I have no idea on how to process the tex file. I tried

quarto render article.tex --to elsevier-pdf

, but error occurs

ERROR: Unable to render article.tex

.

Checklist

[ ] Please include a minimal, fully reproducible example in a single .qmd file? Please provide the whole file rather than the snippet you believe is causing the issue.
[x] Please format your issue so it is easier for us to read the bug report.
[ ] Please document the RStudio IDE version you're running (if applicable), by providing the value displayed in the "About RStudio" main menu dialog?
[X] Please document the operating system you're running. If on Linux, please provide the specific distribution.
[X] Please provide the output of quarto check so we know which version of quarto and its dependencies you're running.

cscheid commented 1 year ago

(I think @dragonstyle is the one most familiar with layout options.)

dragonstyle commented 9 months ago

Adding @cscheid with a todo for us to discuss and get me pointed in the right direction vis-a-vis new panel layout work.

cscheid commented 9 months ago

Oh yeah, I bet I'm the one who knows this one the best now. Thanks, I'll take it over.

cscheid commented 9 months ago

In 1.4, the following syntaxes work as you'd expect:

---
title: issue-5271
keep-tex: true
format:
  pdf:
    fig-pos: 'H'
---

::: {#asdf fig-pos="H" layout-ncol=2}
![](surus.jpg)

![](surus.jpg)
:::

::: {#fig-asdf layout-ncol=2}
![](surus.jpg)

![](surus.jpg)
:::

Note a couple of things. If you use #asdf as the id, then we don't recognize it as a figure, and so you don't get fig-pos placement by default. In that case, you have to add fig-pos="H" explicitly. Note that it's fig-pos, and not fig.pos as in the submitted example (knitr uses dots; quarto uses dashes). If you use #fig-asdf, though, then we recognize it as a figure and use your global figure settings.

giabaio commented 9 months ago

Thanks for this. Can I just ask whether this is supposed to work for computed figures too? I have one based on R/ggplot graph that takes up the whole page and gets sent at the end of the chapter, even if fig-pos is set to H. Basically the resulting .tex file doesn't get parsed with the tag [!h] and in fact this is simply left blank... I can of course force the .tex file and recompile... Am I missing something obvious? I am adding the actual example here (which is predicated upon the R package BCEA though...)

```{r}
#| label: fig-CEplane-vs-ICER
#| echo: false
#| warning: false
#| message: false
#| fig-pos: "H"
#| layout-nrow: 2
#| layout-ncol: 2
#| fig-height: 7
#| fig-width: 6
#| fig-cap: >
#|    Cost-effectiveness plane, showing simulations from the joint
#|    (posterior) distribution of the random variables $(\Delta_e, \Delta_c)$.
#| fig-subcap: 
#|   - ""
#|   - ""
#|   - ""
#|   - ""

library(BCEA)
data(Vaccine)
m <- bcea(eff, cost, ref=2)

par(mgp=c(0,0,0))
plot(unlist(m$delta_e), unlist(m$delta_c), cex=0.6, xlab="Effectiveness differential", ylab="Cost differential",axes=FALSE,pch=20,col="dark grey")
rx <- range(m$delta_e)
ry <- range(m$delta_c)
arrows(0,ry[1],0,ry[2],angle=15,length=.1,lwd=2)    #col="dark grey",
arrows(rx[1],0,rx[2],0,angle=15,length=.1,lwd=2)    #col="dark grey"

par(mgp=c(0,0,0))
plot(unlist(m$delta_e), unlist(m$delta_c),cex=.6,xlab="Effectiveness differential",
     ylab="Cost differential",axes=F,pch=20,col="grey95")
points(mean(unlist(m$delta_e)), mean(unlist(m$delta_c)),pch=20,col="red")
rx <- range(m$delta_e)
ry <- range(m$delta_c)
arrows(0,ry[1],0,ry[2],angle=15,length=.1,lwd=2)    #col="dark grey",
arrows(rx[1],0,rx[2],0,angle=15,length=.1,lwd=2)    #col="dark grey"

par(mgp=c(0,0,0))
rx <- range(m$delta_e)
ry <- range(m$delta_c)
m.e <- rx[1]
M.e <- rx[2]
m.c <- ry[1]
M.c <- ry[2]
step <- (M.e-m.e)/10
wtp <- 25000
m.e <- ifelse(m.e<0,m.e,-m.e)
m.c <- ifelse(m.c<0,m.c,-m.c)
x.pt <- .95*m.e
y.pt <- ifelse(x.pt*wtp<m.c,m.c,x.pt*wtp)
xx <- seq(100*m.c/wtp,100*M.c/wtp,step)
yy <- xx*wtp
xx[1] <- ifelse(min(xx)<m.e,xx[1],2*m.e)
yy[1] <- ifelse(min(yy)<m.c,yy[1],2*m.c)
xx[length(xx)] <- ifelse(xx[length(xx)]<M.e,1.5*M.e,xx[length(xx)])
plot(unlist(m$delta_e), unlist(m$delta_c),xlab="Effectiveness differential",
     ylab="Cost differential",axes=F,col="white")
polygon(c(min(xx),seq(min(xx),max(xx),step),max(xx)),
        c(min(yy),wtp*seq(min(xx),max(xx),step),min(yy)),
        col="grey95",border="black")
points(unlist(m$delta_e), unlist(m$delta_c),cex=.6,pch=20,col="grey70")
points(mean(unlist(m$delta_e)), mean(unlist(m$delta_c)), pch=20,col="red")
arrows(0,ry[1],0,ry[2],angle=15,length=.1,lwd=2)    #col="dark grey",
arrows(rx[1],0,rx[2],0,angle=15,length=.1,lwd=2)    #col="dark grey"

par(mgp=c(0,0,0))
rx <- range(m$delta_e)
ry <- range(m$delta_c)
m.e <- rx[1]
M.e <- rx[2]
m.c <- ry[1]
M.c <- ry[2]
step <- (M.e-m.e)/10
wtp <- 1000
m.e <- ifelse(m.e<0,m.e,-m.e)
m.c <- ifelse(m.c<0,m.c,-m.c)
x.pt <- .95*m.e
y.pt <- ifelse(x.pt*wtp<m.c,m.c,x.pt*wtp)
xx <- seq(100*m.c/wtp,100*M.c/wtp,step)
yy <- xx*wtp
xx[1] <- ifelse(min(xx)<m.e,xx[1],2*m.e)
yy[1] <- ifelse(min(yy)<m.c,yy[1],2*m.c)
xx[length(xx)] <- ifelse(xx[length(xx)]<M.e,1.5*M.e,xx[length(xx)])
plot(unlist(m$delta_e), unlist(m$delta_c),xlab="Effectiveness differential", ylab="Cost differential",axes=F,col="white")
polygon(c(min(xx),seq(min(xx),max(xx),step),max(xx)),
        c(min(yy),wtp*seq(min(xx),max(xx),step),min(yy)),
        col="grey95",border="black")
points(unlist(m$delta_e), unlist(m$delta_c), cex=.6,pch=20,col="grey70")
points(mean(unlist(m$delta_e)), mean(unlist(m$delta_c)),pch=20,col="red")
arrows(0,ry[1],0,ry[2],angle=15,length=.1,lwd=2)    #col="dark grey",
arrows(rx[1],0,rx[2],0,angle=15,length=.1,lwd=2)    #col="dark grey"



Thanks!
Gianluca

mcanouil commented 9 months ago

@giabaio Could you make it simple? Is the figure complexity really required for your example or simply four plot(1) are enough?

giabaio commented 9 months ago

Thanks, @mcanouil --- I suppose I can add the produced (sub)figures and that would work, yes...

mcanouil commented 9 months ago

The thing is you asked us to test your code. Here, that means we have to install dependencies, etc while I don't think the complexity of your plots matters at all in the layout part on the question.

Usually for "Can I do?" type of question it is more efficient to actually try and if it does not work come back with a small, simple and fully reproducible Quarto document which contains only what's needed and nothing more.

giabaio commented 9 months ago

Ah --- sorry, I didn't get what you mean... You can do something like

| label: fig-CEplane-vs-ICER

| echo: false

| warning: false

| message: false

| fig-pos: "H"

| layout-nrow: 2

| layout-ncol: 2

| fig-height: 7

| fig-width: 6

| fig-cap: >

| Cost-effectiveness plane, showing simulations from the joint

| (posterior) distribution of the random variables $(\Delta_e, \Delta_c)$.

| fig-subcap:

| - ""

plot(rnorm(1000)) plot(rgamma(1000)) hist(rnorm(1000,2,5)) plot(rnorm(1000),rt(1000))

That would still get the quirky outcome...

cscheid commented 9 months ago

The reprex is:

---
title: Hello
format: pdf
keep-tex: true
---

```{r}
#| label: fig-CEplane-vs-ICER
#| echo: false
#| warning: false
#| message: false
#| fig-pos: "H"
#| layout-nrow: 2
#| layout-ncol: 2
#| fig-height: 7
#| fig-width: 6
#| fig-cap: >
#|    Cost-effectiveness plane, showing simulations from the joint
#|    (posterior) distribution of the random variables $(\Delta_e, \Delta_c)$.
#| fig-subcap: 
#|   - ""
#|   - ""
#|   - ""
#|   - ""
plot(rnorm(1000))
plot(rnorm(100))
hist(rnorm(1000,2,5))
plot(rnorm(1000),rnorm(1000))

cscheid commented 9 months ago

@cderv This is the output we get from knitr:

::: {#fig-CEplane-vs-ICER .cell layout-nrow="2" layout-ncol="2"}
::: {.cell-output-display}
![](5271_files/figure-pdf/fig-CEplane-vs-ICER-1.pdf){#fig-CEplane-vs-ICER-1 fig-pos='H'}
:::

::: {.cell-output-display}
![](5271_files/figure-pdf/fig-CEplane-vs-ICER-2.pdf){#fig-CEplane-vs-ICER-2 fig-pos='H'}
:::

::: {.cell-output-display}
![](5271_files/figure-pdf/fig-CEplane-vs-ICER-3.pdf){#fig-CEplane-vs-ICER-3 fig-pos='H'}
:::

::: {.cell-output-display}
![](5271_files/figure-pdf/fig-CEplane-vs-ICER-4.pdf){#fig-CEplane-vs-ICER-4 fig-pos='H'}
:::

Cost-effectiveness plane, showing simulations from the joint (posterior) distribution of the random variables $(\Delta_e, \Delta_c)$.

:::

Note that fig-pos is only forwarded to the subimages. We can solve this by guessing that these options probably apply to the outer figure as well, but I think that a better fix would be for knitr to keep fig-pos="H" on the div, as if we had written it explicitly, like this:

::: {#asdf fig-pos="H" layout-ncol=2}
![](surus.jpg)

![](surus.jpg)
:::

Do you think that would be feasible?

cderv commented 9 months ago

Yes that would be feasible. but note that fig-pos is not the only option we do place only on figures.

All those options will be placed only on figures.

https://github.com/quarto-dev/quarto-cli/blob/9736af79aa5c4e7c0f6153b570edccd77208ff2d/src/resources/rmd/hooks.R#L558-L605

They are key values pairs to put as attributes inside the markdown image syntax ![ ]( ){ }

Only fig-cap gets a special treatment when fig-subcap is placed.

This is all because knitr does not know about sub-figures in its option handling, so we need to do that in Quarto (or add support to knitr).

If we keep the logic of fig-subcap needs to be provided for subfigures in chunk, we could definitly check this and set fig-pos on outer div, instead of in each figure. Especially if it does not make sense to put it on each sub figure.

cscheid commented 9 months ago

Thanks for the added context!

I'm not sure about the other options without investigating further, and I'd prefer to fix the bug that was reported right now. We might want to add a 1.5 issue to track the other options, and fix fig-pos for 1.4.

cscheid commented 9 months ago

This was fixed for fig-pos and fig-env by #7972.

giabaio commented 9 months ago

Thank you! And I can confirm that the example that previously wasn't working, it does now!

quarto-dev / quarto-cli

fig-pos doesn't work for figure having subfigures #5271

Bug description

Checklist

| label: fig-CEplane-vs-ICER

| echo: false

| warning: false

| message: false

| fig-pos: "H"

| layout-nrow: 2

| layout-ncol: 2

| fig-height: 7

| fig-width: 6

| fig-cap: >

| Cost-effectiveness plane, showing simulations from the joint

| (posterior) distribution of the random variables $(\Delta_e, \Delta_c)$.

| fig-subcap:

| - ""

| - ""

| - ""

| - ""