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

date_breaks and date_breaks minor don't check argument type #5880

Open
botanize opened this issue May 2, 2024 · 2 comments
Open

date_breaks and date_breaks minor don't check argument type #5880

botanize opened this issue May 2, 2024 · 2 comments
Labels
tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day

Comments

@botanize
Copy link
Contributor

botanize commented May 2, 2024

datetime_scales accepts any argument type for date_breaks and date_minor_breaks, but the parameter documentation claims it only accepts character strings. It's somewhat easy to accidentally supply custom date breaks using scales::breaks_width to the wrong parameter (date_breaks instead of breaks), after all, you're defining date-breaks either way.

I expected an error that the date_breaks parameter had the wrong type, instead I got the unhelpful: Error in strsplit(unitspec, " ") : non-character argument.

ggplot(economics, aes(date, unemploy)) + 
  geom_line() + 
  scale_x_date(date_breaks = scales::breaks_width('10 years', offset = '6 months')
@teunbrand
Copy link
Collaborator

Thanks for the report! If this is a request for better messages, it is probably better to appeal to {scales} to improve the message. ggplot2 has little control over the messages the packages it depend on throw.

@botanize
Copy link
Contributor Author

botanize commented May 2, 2024

I would like better messages, but I think the best way of doing that would be to check that the inputs received by the function are the correct type (as stated by function documentation), instead of passing them blindly to other functions unknown. In this case, it's because I passed a function to the wrong breaks parameter (date_breaks instead of breaks). While I guess {scales} could check inputs as well, this does seem like something that should happen in datetime_scales. Oddly, I see that the default value for the date_breaks parameter in datetime_scales is waiver(), clearly not a character string.

@teunbrand teunbrand added the tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day label May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day
Projects
None yet
Development

No branches or pull requests

2 participants