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

Move name repair to vctrs #464

Closed
hadley opened this issue Aug 22, 2018 · 9 comments · Fixed by #608
Closed

Move name repair to vctrs #464

hadley opened this issue Aug 22, 2018 · 9 comments · Fixed by #608
Labels
vctrs ↗️ Requires vctrs package

Comments

@hadley
Copy link
Member

hadley commented Aug 22, 2018

It seems like maybe name repair is more properly the domain of vctrs, since we'll want to be able to apply the same strategies when concatenating vectors.

@krlmlr
Copy link
Member

krlmlr commented Aug 23, 2018

Not sure. Names have unique problems (e.g. if or TRUE are not symbolic names).

@hadley
Copy link
Member Author

hadley commented Aug 23, 2018

And those problems also arise with lists.

@krlmlr
Copy link
Member

krlmlr commented Aug 23, 2018

Of course, I was confused. Would you like me to submit a PR there? Can we get vctrs to CRAN in the next few weeks?

@hadley
Copy link
Member Author

hadley commented Aug 23, 2018

No, this is a long term project.

krlmlr pushed a commit that referenced this issue Aug 30, 2018
Fixes #463 Exposure (or lack thereof) of syntactic argument in tibble() and as_tibble() 
Fixes #461 Advertise .tidy_names
Fixed #446 Justify tidy_names

---

Other name-related tibble issues

  * Finalize interface of `.name_repair = <A_FUNCTION>` #476 *let's decide/discuss there*
  * tibble doesn't make it easy to get variable names that match our style guide #472 *let's solve in follow-up PR*
  * names<- method should protect against invalid names #466 *let's solve in follow-up PR*
  * Move name repair to vctrs #464 *PR lays groundwork for this long-term goal*
  * revisit strategy for making names syntactic #459 *let's solve in follow-up PR*
  * Re-encode character columns and column names to UTF-8 #87 *relevant, but I don't touch this*

Related issue in [tidyverse/principles](https://github.com/tidyverse/principles):

  * [Vector names](tidyverse/design#4)

---

Overview:

  * Defines and motivates 3 levels of repaired names:
    - `minimal`: not `NULL`, no `NA`s, à la `rlang::names2()`
    - `unique`: `minimal` + no `""`s and no dupes (cf `tidy_names(syntactic = FALSE)`)
    - `syntactic`: `unique` + syntactic (cf `tidy_names(syntactic = TRUE)`)
  * Internal utility functions for each level of name repair, i.e. all combinations of "repair vs check" and "operate on the names vs the named vector".
  * `tibble()` and `as_tibble()` expose name repair via `.name_repair`. Replaces `.tidy_names` (piloted only in dev) and `as_tibble(..., validate = TRUE/FALSE)` (in CRAN version).
  * Default behaviour (`.name_repair = "assert_unique")`): do no repair but check that the names are `unique`.
  * `.name_repair = "none"` means just make sure the names are `minimal`
  * `.name_repair = "unique"` and `.name_repair = "syntactic"` mean what you think
  * `.name_repair` can also take a user-supplied function.

---

To discuss:

  * ~Should `valid` just be `unique`?~ YES and I have made it so in the source.
  * Let us review the "names degrees of freedom" and facilities offered by base, tidyverse for independent control:
    - exist or not
    - unique or not
    - syntactic or not
  * Automatic naming, based on expression. We currently offer no control over this in `tibble()`.
  * How much to reveal re: long-term plans. For example, why `tidy_names()` and `set_tidy_names()` are still here, but haven't gained the obvious counterparts like `unique_names()` and `set_syntactic_names()`. Do we need a Life Cycle section in the docs?
  * My backwards compatibility report
@hadley
Copy link
Member Author

hadley commented Oct 6, 2018

Or possibly rlang. cc @lionel-

@lionel-
Copy link
Member

lionel- commented Oct 6, 2018

Is this connected to r-lib/rlang#651?

@hadley
Copy link
Member Author

hadley commented Oct 7, 2018

Yes, that's part of it, but probably all of the name management strategies should move to rlang.

@krlmlr krlmlr changed the title Move name repair to vctrs Move name repair to rlang Jan 29, 2019
@krlmlr krlmlr changed the title Move name repair to rlang Move name repair to vctrs May 1, 2019
@krlmlr krlmlr added the vctrs ↗️ Requires vctrs package label May 29, 2019
@krlmlr
Copy link
Member

krlmlr commented Jun 6, 2019

Blocked, unclear how to implement "check_unique" .

krlmlr added a commit that referenced this issue Jun 17, 2019
- Delegate name repair to vctrs (#464).
@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2020

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
vctrs ↗️ Requires vctrs package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants