-
Notifications
You must be signed in to change notification settings - Fork 71
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
Keep at most one empty line at the end of each example #880
Conversation
676a8ad
to
966b092
Compare
I am not sure I can follow. Can you provide a new test to demonstrate the expected behavior? I.e. in and out files? |
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 5e82878 is merged into main:
Further explanation regarding interpretation and methodology can be found in the documentation. |
Also, we don't squash examples anymore: #747 |
Thanks. The "examples" are tibble and pillar -- try styling e.g. |
Ok, I see. But what's the rationale of having a blank line like between roxygen comments this? From tibble, ( #' Add columns to a data frame
#'
#' This is a convenient way to add one or more columns to an existing data
#' frame.
#'
#' @param .data Data frame to append to.
#' @param ... <[`dynamic-dots`][rlang::dyn-dots]>
#' Name-value pairs, passed on to [tibble()]. All values must have
#' the same size of `.data` or size 1.
#' @param .before,.after One-based column index or column name where to add the
#' new columns, default: after last column.
#' @inheritParams tibble
#' @family addition
#' @examples
#' # add_column ---------------------------------
#' df <- tibble(x = 1:3, y = 3:1)
#'
#' df %>% add_column(z = -1:1, w = 0)
#' df %>% add_column(z = -1:1, .before = "y")
#'
#' # You can't overwrite existing columns
#' try(df %>% add_column(x = 4:6))
#'
#' # You can't create new observations
#' try(df %>% add_column(z = 1:5))
#'
#' @export
add_column |
No, the blank line is bogus and can be removed. This is about the empty line before |
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 8d11f22 is merged into main:
Further explanation regarding interpretation and methodology can be found in the documentation. |
I have updated the test output files, there's still trouble. Could you please take a look? Also, there are "unknown or unitialised token column" warnings, I have seen them before with CRAN styler. I do wonder why this change adds so many empty lines for some outputs, in particular in the presence of |
Empty lines look good to me now. Would you support this change? |
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 83983f8 is merged into main:
Further explanation regarding interpretation and methodology can be found in the documentation. |
Regarding the tidyverse style guide: not sure, it just feels that examples would be too condensed without empty lines inbetween. |
If I understand you correctly from your last three comments, all we need is to keep an empty line at the end of an example section if already one or more empty lines are present (empty as in only |
Yes, that would be great.
On 23 Dec 2021 at 12:51:14 CET, Lorenz Walthert ***@***.***> wrote:
If I understand you correctly from your last two comments, all we need is to keep an empty line at the end of an example section if already one or more empty lines are present (empty as in only #' on that line, not completely empty). If there is none, we should not add one. Correct? —
Reply to this email directly, view it on GitHub , or unsubscribe .
Triage notifications on the go with GitHub Mobile for iOS or Android .
You are receiving this because you authored the thread. Message ID: <r-lib/styler/pull/880/c1000251592 @ github . com>
|
c67d0bd
to
bfe2e82
Compare
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 4f73008 is merged into main:
Further explanation regarding interpretation and methodology can be found in the documentation. |
The warning about unknown or uninitialized column seems to happen in various places in the code (as you can tell from the warnings). And it's not new. Probably it's similar to this: x <- tibble::tibble()
x$ff[logical()] <- 1
#> Warning: Unknown or uninitialised column: `ff`. Created on 2021-12-23 by the reprex package (v2.0.1) For some reason, Also, don't modify the |
@krlmlr can you have a look at the new test case I added and if that's what you want, you can merge it 🥳 |
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.
Great, thanks!
Are you suggesting that x <- tibble::tibble(ff = numeric())
x$ff[logical()] <- 1
x
#> # A tibble: 0 × 1
#> # … with 1 variable: ff <dbl> Created on 2021-12-24 by the reprex package (v2.0.1) We should normalize the data early in the process so that the structure is stable. |
Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 448a7a8 is merged into main:
Further explanation regarding interpretation and methodology can be found in the documentation. |
Well, it has columns. But I think the problem is that it does not habe the ones to which something is assigned. Comparing my example with yours, yours already has the column that is assigned to. I’ll open a nee issue. |
This is crucial for combining multiple
@examples
and@examplesIf
elements.