-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Consolidate coordinate scale expansion and limiting code #3380
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
paleolimbot
merged 26 commits into
tidyverse:master
from
paleolimbot:issue-3371-expantion-strategy
Jul 1, 2019
Merged
Consolidate coordinate scale expansion and limiting code #3380
paleolimbot
merged 26 commits into
tidyverse:master
from
paleolimbot:issue-3371-expantion-strategy
Jul 1, 2019
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…expansion.r add and test the new scale_dimension() function
hadley
reviewed
Jun 24, 2019
…s in scale-expansion.r
This was referenced Jun 25, 2019
Closed
hadley
approved these changes
Jul 1, 2019
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a PR to address coordinate scale expansion and limiting using a single approach, implementing several feature requests and fixing several bugs in the process (closes #2990, closes #2907, closes #3056, closes #3336). The inspiration for this PR was a collection of issues that @hadley collected when issue triaging that were related to the interface between scales and coordinate systems (#3371). I'd be happy to split this up into a few different PRs if it is easier to review (one for expansion, one for coord limiting, one for sprucing up coord_trans).
All scale expansion code now lives in
scale-expansion.r
, and is tested independently of the scales/coords. AllCoord
objects now use these expansion functions. Previously, some form of scale expansion code lived inutilities.r
,scale-.r
,coord-sf.r
,coord-cartesian.r
,scale-discrete.r
, andcoord-transform.r
. This led to at least one issue of inconsistent expansion between coords (Remove extra space created bycoord_trans
#3338), and was thought to cause several other issues, although it turns out that Secondary axis doesn't show axis ticks or labels with coord_trans #2990 was unrelated, Scale expand value from scales ignored when coord_cartesian ylim is used #3270 had already been fixed before this PR, and nothing about this PR addressed sqrt_trans, scale limit expansion, and missing breaks #980.All
coord_*()
functions withxlim
andylim
arguments now accept vectors withNA
as a placeholder for the minimum or maximum value (e.g.,ylim = c(0, NA)
would zoom the y-axis from 0 to the maximum value observed in the data). This mimics the behaviour of thelimits
argument in continuous scale functions (NA
limits in coords #2907).coord_trans()
now draws second axes (Secondary axis doesn't show axis ticks or labels with coord_trans #2990) and acceptsxlim
,ylim
, andexpand
arguments to bring it up to feature parity withcoord_cartesian()
. This also closes Deprecatelimx
andlimy
incoord_trans()
#3056, which is an abandoned PR that deprecatedlimx
andlimy
in favour ofxlim
andylim
. This PR does the same, and also fully removesxtrans
andytrans
, which have been deprecated long enough that using them resulted in an error anyway.coord_trans()
now calculates breaks using the expanded range (previously these were calculated using the unexpanded range, which resulted in differences between plots made withcoord_trans()
and those made withcoord_cartesian()
. The expansion for discrete axes incoord_trans()
was also updated such that it behaves identically to that incoord_cartesian()
(see Remove extra space created bycoord_trans
#3338 - this was already closed by the OP, but it is the best discussion of this topic).coord_trans()
, for which previously there were none.