Skip to content

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

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

Merged
merged 16 commits into from
Jun 3, 2023

Conversation

dereckmezquita
Copy link
Contributor

@dereckmezquita dereckmezquita commented Apr 9, 2023

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)

@dereckmezquita
Copy link
Contributor Author

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

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]>
dereckmezquita and others added 2 commits June 3, 2023 09:55
@cpsievert
Copy link
Collaborator

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

@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