[x] added unit tests and checked code coverage with covr::report() (should aim for 100%)
[x] ran devtools::document()
[x] ran lintr::lint_package() and resolved all lint warnings and notes
[x] ran styler::style_pkg() to make sure code matches the style guidelines
[x] ran R-CMD CHECK and resolved all issues
This pull request provides code that simplifies the if-else logic currently in the ptd_create_ggplot() function.
It does this in two ways:
By providing a fallback value (ggplot2::waiver()) for the y_axis_labels object, it allows a whole layer of logic that currently handles different paths for NULL/non-NULL values for y_axis_breaks to be removed
It allows the built-in accuracy heuristic provided in the scales::label_percent() and scales::label_number() functions to do its job, without us hard-coding the accuracy levels based on the value of y_axis_breaks (which doesn't really make sense anyway).
It does still allow hard-coding of accuracy = 0.1 for the secondary axis on percentage charts.
Though this could also be removed in future if thought to be the right thing to do.
This PR closes issue #210 and also closes issue #211 by replacing the trans argument to sec_axis() with the new argument name transform.
There is an associated test that no longer passes with these changes. This test does not look to actually test what it says it does, however - this needs further attention perhaps.
There is also a re-formatting of the hex_logo function under inst/, but with no change in functionality.
(These changes are unrelated to issues 210 or 211).
tomjemmett
commented
3 weeks ago
covr::report()(should aim for 100%)devtools::document()lintr::lint_package()and resolved all lint warnings and notesstyler::style_pkg()to make sure code matches the style guidelinesThis pull request provides code that simplifies the if-else logic currently in the ptd_create_ggplot() function. It does this in two ways:
ggplot2::waiver()) for they_axis_labelsobject, it allows a whole layer of logic that currently handles different paths for NULL/non-NULL values fory_axis_breaksto be removedscales::label_percent()andscales::label_number()functions to do its job, without us hard-coding the accuracy levels based on the value ofy_axis_breaks(which doesn't really make sense anyway).It does still allow hard-coding of
accuracy = 0.1for the secondary axis on percentage charts. Though this could also be removed in future if thought to be the right thing to do.This PR closes issue #210 and also closes issue #211 by replacing the
transargument tosec_axis()with the new argument nametransform.There is an associated test that no longer passes with these changes. This test does not look to actually test what it says it does, however - this needs further attention perhaps.
There is also a re-formatting of the hex_logo function under
inst/, but with no change in functionality. (These changes are unrelated to issues 210 or 211).