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

Expose vec_names2(), vec_names() and vec_set_names() #1173

Merged
merged 4 commits into from
Jul 6, 2020
Merged

Conversation

Copy link
Member

@lionel- lionel- left a comment

Choose a reason for hiding this comment

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

Also needs a NEWS bullet.

R/names.R Outdated
#' Returns the repaired names from a vector, even if the vector is unnamed.
#' @description
#' These functions work like [rlang::names2()], [names()] and [names<-()],
#' except that they return or modify the conceptual names of the vector.
Copy link
Member

Choose a reason for hiding this comment

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

the rowwise names of the vector. These are:

  • The usual names() for atomic vectors and lists
  • The row names for data frames and matrices
  • The names of the first dimension for arrays

Copy link
Member

Choose a reason for hiding this comment

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

Excuse the drive-by comment, but what is meant by "conceptual" names? If that term is defined and used elsewhere and I just don't have enough context, then never mind.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed this is not a defined term. I'm suggesting to replace "conceptual" by "rowwise", which seems clearer?

Copy link
Member

Choose a reason for hiding this comment

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

I think since it is so related to the vctrs concept of size, they could be called "size consistent names"

R/names.R Outdated Show resolved Hide resolved
R/names.R Outdated Show resolved Hide resolved
R/names.R Outdated
#' `vec_names()` is a bare-bones version that returns `NULL` if the vector is
#' unnamed.
#'
#' `vec_set_names()` sets the names.
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps a note that you can set names to NULL to remove them?

krlmlr and others added 2 commits July 6, 2020 15:14
Co-authored-by: Davis Vaughan <davis@rstudio.com>
Co-authored-by: Lionel Henry <lionel.hry@gmail.com>
@krlmlr krlmlr requested a review from lionel- July 6, 2020 13:23
@krlmlr
Copy link
Member Author

krlmlr commented Jul 6, 2020

Thanks for the feedback!

Comment on lines +204 to +205
#' Rowwise names are size consistent: the length of the names always equals
#' [vec_size()].
Copy link
Member

Choose a reason for hiding this comment

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

👍

@lionel- lionel- merged commit 7a86784 into master Jul 6, 2020
@lionel-
Copy link
Member

lionel- commented Jul 6, 2020

Thanks!

@lionel- lionel- deleted the f-expose-names branch July 6, 2020 13:35
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.

4 participants