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

Should the trans argument in scale_*_continuous etc be renamed to transform? #5558

Closed
davidhodge931 opened this issue Dec 2, 2023 · 9 comments · Fixed by #5566
Closed

Should the trans argument in scale_*_continuous etc be renamed to transform? #5558

davidhodge931 opened this issue Dec 2, 2023 · 9 comments · Fixed by #5566

Comments

@davidhodge931
Copy link

Given that the scales functions will be changing from scales::*_trans to scales::transform_*

@teunbrand
Copy link
Collaborator

I'm not sure whether that kerfuffle associated with changing argument names is worth the small increase in consistency.

@davidhodge931
Copy link
Author

There must have been a reason for using scales::transform_* rather than scales::trans_*.

Presumably the same reason holds? Consistency makes things easier

@teunbrand
Copy link
Collaborator

I'll ask @thomasp85 to weigh in on this decision

@yutannihilation
Copy link
Member

yutannihilation commented Dec 4, 2023

I'm not sure about the details, but the NEWS says:

Transformation function have been renamed to transform_*-prefixed names instead of *_trans-suffixed names. This allows for a better tab-completion search of transformations.

Regarding tab-completion, I don't see any benefit in ggplot2's case.

@thomasp85
Copy link
Member

The main reason for the scales change was indeed to streamline the naming conventions in the package to have the common name in front to make tab-completion easier. However, as we were already renaming things we decided to also use a more descriptive and inclusive naming.

Let's keep this open but it is unlikely to be included in the upcoming release

@eliocamp
Copy link
Contributor

eliocamp commented Dec 5, 2023

I like the trans argument name 🏳️‍⚧️

@yutannihilation
Copy link
Member

Just curious, should coord_trans() also be renamed for consistency?

@teunbrand
Copy link
Collaborator

I like the consistency argument, and I think in a perfect world that would be ideal. However, spending some time with untangling the latest reverse depency checks, I am usure we should even rename the Scale$trans field to Scale$transform. Probably the deprecation of the old arguments should happen in a more gradual fashion than the RC branch brings now. However, it would be easy to code coord_transform() as an alias for coord_trans(), so I can get behind this.

@yutannihilation
Copy link
Member

Thanks. I have no intention to push coord_transform(). I too am not sure if the renaming is worth. If there's no strong reason, maybe we can just leave coord_trans() as it is.

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 a pull request may close this issue.

5 participants