Skip to content

redo warnings for overwritten arguments in abline, hline, and vline #3286

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

paleolimbot
Copy link
Member

This is a pull request to address #3090, and in part overwrites the work from #2950. This PR ensures that:

  • An informative warning is issued when mapping and one or more of yintercept/xintercept/slope/intercept are provided.
  • An informative warning is issued when data and one or more of yintercept/xintercept/slope/intercept are provided.

It is also worth noting that I changed the test for "data is provided" from missing(data) to is.null(data). I am not sure if there was a good reason for it to have been missing(data) in the first place, as something like

data <- NULL
geom_hline(yintercept = 3, data = data)

would possibly not issue a warning.

I may have gone overboard on the gramatically-correct informative warning message function (warn_ovewritten_args()), but was trying make all the messages informative without duplicating code.

@paleolimbot paleolimbot requested a review from thomasp85 May 3, 2019 19:06
Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just needs a NEWS bullet

@paleolimbot
Copy link
Member Author

Should I add the NEWS bullet here? It doesn't look like there's a set of bullet points started for post-3.2.0.9000 and I think the NEWS.md on my branch is out of date.

@thomasp85
Copy link
Member

Is this intended for 3.2.0?

@paleolimbot
Copy link
Member Author

No, it wasn't.

@thomasp85
Copy link
Member

Ah. Just make a new heading and a bullet then

@paleolimbot paleolimbot merged commit a908fe7 into tidyverse:master Jun 4, 2019
@paleolimbot paleolimbot deleted the issue-3090-warn-data-hline-vline-abline branch June 4, 2019 16:54
@lock
Copy link

lock bot commented Dec 1, 2019

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/

@lock lock bot locked and limited conversation to collaborators Dec 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants