Skip to content

Bug fix: allow empty Vlines and Hlines as ggplot2 does - #2252

Merged
cpsievert merged 16 commits into
plotly:masterfrom
dereckmezquita:master
Jun 3, 2023
Merged

cpsievert merged 16 commits into
plotly:masterfrom
dereckmezquita:master

Conversation

@dereckmezquita

@dereckmezquita dereckmezquita commented Apr 9, 2023 •

Copy link
Copy Markdown
Contributor

Vlines and Hlines might sometimes be inexistent; ggplot2 allows this, but plotly fails on merging empty data.frame.

I believe my solution is appropriate. I tested it locally, but if someone can help me through the process of writing a formal unit test, I would appreciate it.

Close #1947

# prep data
data <- data.table::data.table(gapminder::gapminder)
# add year of interest for vertical lines
data[year == 2002, year_of_interest := "yoi"]

# normal vertical line set manually at year 2000
ggplot2::ggplot(data, ggplot2::aes(x = year, y = lifeExp, colour = country)) +
    ggplot2::geom_line() +
    ggplot2::geom_vline(xintercept = 2000, colour = "red") +
    ggplot2::theme(legend.position = "none")

# vertical line by feeding data to it; this allows for programmatically setting many lines at different years
p <- ggplot2::ggplot(data, ggplot2::aes(x = year, y = lifeExp, colour = country)) +
    ggplot2::geom_line() +
    ggplot2::geom_vline(
        ggplot2::aes(xintercept = year),
        data = data[year_of_interest == "yoi", ],
        colour = "yellow",
        alpha = 0.75,
        linewidth = 1.25,
        linetype = "dashed"
    ) +
    ggplot2::theme(legend.position = "none")


# ggplot2 accepts that no lines are found plot is created and no error is thrown
p <- ggplot2::ggplot(data, ggplot2::aes(x = year, y = lifeExp, colour = country)) +
    ggplot2::geom_line() +
    ggplot2::geom_vline(
        ggplot2::aes(xintercept = year),
        data = data[year_of_interest == "some_not_found", ],
        colour = "yellow",
        alpha = 0.75,
        linewidth = 1.25,
        linetype = "dashed"
    ) +
    ggplot2::theme(legend.position = "none")

print(p)

# if we pass to plotly, it throws an error
plotly::ggplotly(p)

Comment thread R/layers2traces.R Resolved
Comment thread R/layers2traces.R Resolved
@dereckmezquita

Copy link
Copy Markdown
Contributor Author

@cpsievert does this PR still require more work? Let's get it merged?

Comment thread tests/testthat/test-ggplot-hline.R Resolved
Comment thread tests/testthat/test-ggplot-vline.R Resolved
dereckmezquita and others added 2 commits June 3, 2023 09:35
Co-authored-by: Carson Sievert <[email protected]>
Co-authored-by: Carson Sievert <[email protected]>
Comment thread tests/testthat/test-ggplot-hline.R Resolved
dereckmezquita and others added 2 commits June 3, 2023 09:55
@cpsievert

Copy link
Copy Markdown
Collaborator

Thank you, this is looking good, will you please also update the NEWS?

Comment thread R/layers2traces.R Resolved
@cpsievert
cpsievert merged commit bcf2a21 into plotly:master Jun 3, 2023
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 this pull request may close these issues.

ggplotly fails with an empty layer of geom_vline

3 participants