Skip to content

Replace sf_rescale01_x() with scales::rescale() #5475

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
merged 2 commits into from
Oct 30, 2023

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Oct 11, 2023

This PR fixes a personal pet peeve of mine.

First, let me start by apologising for the tone of this PR.
The two functions in the title are in principle doing the same operation, but scales::rescale() supports more data types than just numeric.
Every time I look at coord_sf()'s code, I have to remind myself that they're really doing the same thing despite being named differently. If this PR gets merged, this saves me 5 seconds of my life every time I'm dealing with coord_sf().

Ulterior motive is also that I might want to feed not-bare-numeric data to coord_sf() at some point with a custom rescale method.

Typical coords use the Scale$rescaler() method. However, as the geometry column cannot be transformed by rescale(), we should not use the Scale$rescaler() method. If we did, annotations such as graticules and axis positions can become out of sync when a user has somehow engineered a non-standard rescaler in a position scale.

@thomasp85
Copy link
Member

Can you see any indication of the need for the old function?

@teunbrand
Copy link
Collaborator Author

I don't think sf_rescale01_x() is needed. It was introduced in 379df62, I think as a pointwise equivalent to sf_rescale01(), which in turn is just scales::rescale().

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 teunbrand merged commit c4a337a into tidyverse:main Oct 30, 2023
@teunbrand teunbrand deleted the remove_sf_rescale01_x branch October 30, 2023 09:37
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.

2 participants