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

Fix for #409 in transition_reveal() causes some rows to be dropped when there are multiple groups in a panel #480

Closed
linzi-sg opened this issue Apr 1, 2023 · 1 comment

Comments

@linzi-sg
Copy link

linzi-sg commented Apr 1, 2023

Situation

After fixing #409 in 1.0.8, TransitionReveal$expand_panel returns

all_frames[!(c(diff(all_frames$.time), 1) <= .Machine$double.eps & all_frames$.phase == 'raw'), ]

instead of

all_frames

Problem

When there are, for example, 2 groups in a dataset, the latter will include 2 rows with .phase = "raw" and .time == max(.time), while the former will only include 1. Consequently, the last frame of the animated plot will only show the geom(s) for one group, instead of both.

A problem of this nature was recently described on Stack Overflow. Based on my attempts to reproduce the issue, the above change appears to be responsible.

Proposed workaround

Modifying the code to drop rows by group (instead of by panel) appears to resolve the issue. I tried changing last few rows from:

all_frames$group <- paste0(all_frames$group, '<', all_frames$.frame, '>')
all_frames$.frame <- NULL
all_frames[!(c(diff(all_frames$.time), 1) <= .Machine$double.eps & all_frames$.phase == 'raw'), ]

to:

all_frames <- do.call("rbind", 
                      lapply(split(all_frames, all_frames$group), 
                             function(d) d[!(c(diff(d$.time), 1) <= .Machine$double.eps & d$.phase == "raw"), ]))

all_frames$group <- paste0(all_frames$group, "<", all_frames$.frame, ">")
all_frames$.frame <- NULL
all_frames

p.s. my original suggestion on SO corresponding to #409 was

all_frames %>%
  filter(!(.phase == "transition" & 
               abs(x - lag(x)) <= sqrt(.Machine$double.eps) &
               abs(y - lag(y)) <= sqrt(.Machine$double.eps)))

Which also seems to work fine, at least for this case.

@linzi-sg
Copy link
Author

linzi-sg commented Apr 1, 2023

I also looked at #473. Both suggestions above worked for the example provided by @ddeannd, but only the 2nd version worked for the ampersand example provided by @yutannihilation. (I couldn't get the Japanese font to work properly, so I skipped his first example...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants