Skip to content

Fixes #4399 #4823

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

Merged
merged 3 commits into from
Apr 30, 2022
Merged

Fixes #4399 #4823

merged 3 commits into from
Apr 30, 2022

Conversation

thoolihan
Copy link
Contributor

Clearer documentation indicating that fullrange may extend beyond the data but does not fill the area created by expansion.

Docs regenerated by devtools::document() did create some warnings that are unrelated to these changes. Presumably known issues?

@yutannihilation
Copy link
Member

some warnings that are unrelated to these changes

Ah, I think you saw this kind of warning. You need to install the packages to generate the proper links. Typically this can be done by remotes::install_deps(), but it seems interp package and akima package are exceptions. Please ignore for now. Sorry for your confusion.

==> devtools::document(roclets = c('rd', 'collate', 'namespace'))

ℹ Updating ggplot2 documentation
ℹ Loading ggplot2
Warning: [C:\Users\Yutani\Documents\GitHub\ggplot2\R\geom-contour.r:3] @description Link to unavailable package: interp::interp. there is no package called 'interp'
Warning: [C:\Users\Yutani\Documents\GitHub\ggplot2\R\geom-contour.r:3] @description Link to unavailable package: akima::bilinear. there is no package called 'akima'
Writing NAMESPACE
Writing NAMESPACE
Documentation completed

But, I believe man/hmisc.Rd should not be changed. Hmisc package was updated very recently, so maybe your Hmisc is not up-to-date?

Clearer documentation indicating that fullrange may extend beyond the data but does not fill the area created by expansion.

Docs regenerated by devtools::document() did create some warnings that are unrelated to these changes. Presumably known issues?
@thoolihan
Copy link
Contributor Author

Thanks for the help, @yutannihilation. I updated Hmisc and the pull request to not include those changes. All tests have passed

Copy link
Member

@yutannihilation yutannihilation left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good, but let me add some nitpicky comments.

  1. Could you add ` around TRUE?
  2. Using the word fullrange in the description of fullrange feels a bit redundant to me. What about the form of "is expanded?" (Sorry, I'm not very fluent in English, so I might be wrong here)

wording change

Co-authored-by: Hiroaki Yutani <yutani.ini@gmail.com>
@thoolihan
Copy link
Contributor Author

I committed the changes, but need to regenerate docs, will push an update and then it should be good

@thoolihan
Copy link
Contributor Author

Docs regenerated, tests passed

@yutannihilation yutannihilation merged commit 3dbbce4 into tidyverse:main Apr 30, 2022
@yutannihilation
Copy link
Member

Thanks!

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.

2 participants