-
Notifications
You must be signed in to change notification settings - Fork 633
Fix "partial argument match" warnings (#1977) #2046
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 "partial argument match" warnings (#1977) #2046
Conversation
@bersbersbers Thanks for the time you put on this. For the tests you can try calling using the code that was in the issue without loading the packages and use something like test_that("a message describing the test", {
# initialising the data and the plot p and set the options
# the actual assertion
expect_warning(ggplotly(p), regexp = NA)
})
|
@moutikabdessabour thanks for the explanation, that was very helpful. I also added a very short test now. |
@bersbersbers can you please revert the changes to the other test files. Otherwise this looks good to me. Thanks again for your time. |
@moutikabdessabour I did it. I think. |
Great. I will wait until @cpsievert approves the PR and I will merge. Again thanks for your time. |
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.
Please update NEWS.md
accordingly
@cpsievert thanks - done! |
@moutikabdessabour done :) |
@bersbersbers can you please use rebase instead of the cmerge master commit". Once it's done I will merge the PR. Again thanks for your time and effort. |
@moutikabdessabour I did it :) |
@bersbersbers I can't thank you enough for this. |
For the first version of this PR, I am most of all interested in if/how CI works.
I will then fix more of these warnings in later commits.
Finally, I wonder how tests should be adapted for this case. On my system, with
options(warnPartialMatchArgs = TRUE)
in.Rprofile
, several existing tests are issuing warnings. One could usein the setup of any test, but that might fail if dependencies issue such warnings.