Skip to content

[WIP] Temporary fix for test-plot-coordmap.R test (Fixes #2169) #2184

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

Closed
wants to merge 2 commits into from

Conversation

schloerke
Copy link
Collaborator

@schloerke schloerke commented Sep 16, 2018

Fixes #2169

Removed ggplot2 remote as it's not needed anymore.


Test now allows for different values of ggplot2 domain. THIS IS A DUCT TAPE FIX for the dev version of ggplot2.

Originally caused by tidyverse/ggplot2#2832

Need to wait for tidyverse/ggplot2#2821 to be merged.

Expecting a bug fix release between 3.0.0 and when this bug will be fixed, so can not compare against pkg versions. :-/

…ersion of ggplot2

Originally caused by tidyverse/ggplot2#2832

Need to wait for tidyverse/ggplot2#2821 to be merged.

Expecting a bug fix release between 3.0.0 and when this bug will be fixed, so can not compare against pkg versions. :-/
@schloerke schloerke self-assigned this Sep 16, 2018
@schloerke schloerke requested a review from jcheng5 September 16, 2018 20:55
@schloerke schloerke added this to the 1.2 milestone Sep 16, 2018
@schloerke
Copy link
Collaborator Author

schloerke commented Sep 17, 2018

Looking at it with Winston, tidyverse/ggplot2#2821 will rename the coord-trans range to backtransform_range, which should solve summarise_layout output not providing fully transformed range values

@schloerke schloerke changed the title Temporary fix for test-plot-coordmap.R test Temporary fix for test-plot-coordmap.R test (Fixes #2169) Sep 17, 2018
@jcheng5
Copy link
Member

jcheng5 commented Sep 18, 2018

To be clear, we shouldn't merge this, as the fix for this problem will land on the ggplot2 master branch. This problem never existed with any CRAN version of ggplot2; we were seeing CI errors because we had Remotes:tidyverse/ggplot2 (which I've now removed).

However, keeping this open to track the issue until the ggplot2 PR above is merged, then we can close this one.

Copy link
Member

@jcheng5 jcheng5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't merge

@schloerke schloerke changed the title Temporary fix for test-plot-coordmap.R test (Fixes #2169) [WIP] Temporary fix for test-plot-coordmap.R test (Fixes #2169) Sep 28, 2018
@schloerke
Copy link
Collaborator Author

fixed in current ggplot2 github branch

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.

ggplot2 coordmap test failure with development version of ggplot2
2 participants