-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix formals of compute_layer() #3244
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
Fix formals of compute_layer() #3244
Conversation
NEWS.md
Outdated
@@ -121,6 +121,9 @@ core developer team. | |||
|
|||
* `sec_axis()` now accepts functions as well as formulas (@yutannihilation, #3031). | |||
|
|||
* `Stat$compute_layer()` and `Position$compute_layer()` now have the same names of | |||
arguments (@yutannihilation, #3202). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel something is missing here. "have now the same names of arguments as other compute_layer()
functions"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I should probably say something like "The arguments of Stat*$compute_layer()
and Position*$compute_layer()
are now renamed to match the ones of Stat$compute_layer()
and Position$compute_layer()
." Does this make sense...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, maybe with this edit: "...now renamed to always match the ones...". After all, they mostly were matching already, but not always.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…pute-layer-consistent-args
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/ |
Fixes #3202
Stat*$compute_layer()
andPosition*$compute_layer()
have various names of formals, which seems for no valid reason; just because the common formals were so at that time (c.f. #3202 (comment)).It seems these methods are not passed parameters by name, but by positions only, so I think this is safe to fix:
This PR fixes the inconsistency. I expect this would break nothing, but changing the argument names is (technically) a breaking change. So, I added a NEWS item for this.