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

Sort scale and viewscale limits #5921

Closed
wants to merge 3 commits into from

Conversation

teunbrand
Copy link
Collaborator

This PR aims to fix #5918.

Briefly, continuous_scale() now sorts the limits and position guides get passed sorted ranges.

The order of the limits should now only matter when choosing e.g. limits = c(NA, 10) to substitute the NA for the data limits. Examples below are from the issue.

Regular scale, flipped limits:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

ggplot() + scale_x_continuous(limits = c(10, 0))

Reverse scale, sorted limits:

ggplot() + scale_x_reverse(limits = c(0, 10))

Created on 2024-05-31 with reprex v2.1.0

@thomasp85
Copy link
Member

I might misremember but wasn't scale reversing by supplying a reversed limit a thing/feature? If so, this would break it, right?

@teunbrand
Copy link
Collaborator Author

teunbrand commented Jul 11, 2024

This was discussed a little bit in the linked issue (TL;DR: it isn't properly documented in ggplot2), but yes this would definitly break such thing if it were a feature.

@thomasp85
Copy link
Member

okay - I'm a bit uneasy about this, just because I anticipate it will break many peoples code (many is maybe a bit too strong, but a non-negligible number)

@teunbrand
Copy link
Collaborator Author

Yeah that's fair, but if we decide that is a proper feature, then it should be documented somewhere and implemented consistently.

@thomasp85
Copy link
Member

Yeah, I agree. We can discuss at a meeting where to go with this

@teunbrand
Copy link
Collaborator Author

Closing this in favour of #6070

@teunbrand teunbrand closed this Sep 6, 2024
@teunbrand teunbrand deleted the sorted_limits branch September 6, 2024 07:39
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.

scale_*_continuous fails to render breaks if axis is reversed
2 participants