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

Free scales for coord_polar() #5354

Merged
merged 7 commits into from
Dec 15, 2023
Merged

Conversation

teunbrand
Copy link
Collaborator

This PR aims to fix #2815.

Briefly, it implements the suggestion to have CoordPolar$is_free() return TRUE.

To deal with the aspect ratio, the requirement for fixed scales to have Coords affect the aspect ratio is dropped. I have so far failed to come up with examples where this may lead to awkward situations, so I hope I'm avoiding Chesterton's Fence here. This requirement was originally implemented in cd11888, but coord_equal()'s behaviour appears unaffected by this PR.

Also, a complication arose in #2815 (comment). With this PR, coord_equal() can also have free scales in facet_grid(), but these are of course locked to the column for x-axes and to the rows for y-axes (see also #1344 and #1152, which might be a topic revisiting separately from this PR). Because theta can be either axis, it has no special relation with either horizontal or vertical directions, which makes this a bit awkward.

@thomasp85
Copy link
Member

is this tangentially related enough to the milestone for inclusion?

@teunbrand
Copy link
Collaborator Author

Might be nice, but should double check if this holds for coord_radial() as well

Merge branch 'main' into free_polar

# Conflicts:
#	tests/testthat/test-coord-polar.R
@teunbrand
Copy link
Collaborator Author

Yeah lets include this in the milestone, why not?

@teunbrand teunbrand added this to the ggplot2 3.5.0 milestone Dec 14, 2023
@thomasp85
Copy link
Member

is it ready for review?

@teunbrand
Copy link
Collaborator Author

Yes, I'll request it

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

LGTM

@teunbrand
Copy link
Collaborator Author

Thanks!

@teunbrand teunbrand merged commit acf3641 into tidyverse:main Dec 15, 2023
12 checks passed
@teunbrand teunbrand deleted the free_polar branch December 15, 2023 08:11
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.

Awkward interaction between faceting, polar coordinates, and free scales
2 participants