Skip to content

Document flipped computed variables #4128

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

Conversation

yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Jul 11, 2020

Fix #4124

These four stat have variables whose names differ depending on the orientation.

  • stat_boxplot(): middle, lower, and upper. This feels a bit tricky at least in two points; the flipped names are asynmetric (e.g. middle vs xmiddle, not ymiddle vs xmiddle), and notchlower and notchupper doesn't have their flipped form.
  • stat_smooth(): y, ymin, and ymax
  • stat_function() stat_ecdf() : these generate both x and y, so the documentation would be cryptic like below, so I don't address them in this pull request (at least they don't have undocumented computed variables)
#' `stat_function()` computes the following variables:
#' \describe{
#'   \item{x or y}{x or y values along a grid}
#'   \item{y or x}{value of the function evaluated at corresponding x or y}
#' }

edit: I just remembered stat_ecdf() doesn't support flipping (c.f. #4005)

@yutannihilation
Copy link
Member Author

Added this sentence. Does this sound good?

stat_boxplot() provides the following variables, some of which depend on the orientation:

@clauswilke
Copy link
Member

I think this is fine. Maybe put the "or" in italics just as is the case for the aesthetics.

@clauswilke
Copy link
Member

The "or" between computed variables, I mean.

@yutannihilation
Copy link
Member Author

Makes sense, I put them in Italic.

@yutannihilation yutannihilation merged commit 2ac58ca into tidyverse:master Jul 12, 2020
@yutannihilation yutannihilation deleted the fix/issue-4124-document-flipped-stat branch July 12, 2020 01:18
Copy link
Member

@clauswilke clauswilke left a comment

Choose a reason for hiding this comment

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

LGTM

@clauswilke
Copy link
Member

Oh, this was merged already. Looks like my browser had cached an old version.

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.

Bug in setting fill of horizontal boxplots
2 participants