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

estvarname cannot be named est #49

Closed
mikesweeting opened this issue Mar 8, 2023 · 2 comments
Closed

estvarname cannot be named est #49

mikesweeting opened this issue Mar 8, 2023 · 2 comments
Assignees
Milestone

Comments

@mikesweeting
Copy link

Describe the bug
estvarname cannot be called "est". An easy fix is just to call it something else, but am reporting this anyway for completeness!

data("nlp", package = "rsimsum")
nlp.rename <- nlp %>% rename(est = b)
s.nlp.estvarname <- rsimsum::simsum(
  data = nlp.rename, estvarname = "est", true = 0, se = "se",
  methodvar = "model", by = c("baseline", "ss", "esigma")
)
@ellessenne ellessenne self-assigned this May 27, 2023
@ellessenne
Copy link
Owner

Hi @mikesweeting,

This is intended behaviour, as I am using specific names internally to do calculations.
I thought about updating the names internally to use something that is less likely to "collide" with user-defined column names, but I realised this might break a lot of other users' code too – so I'd prefer not changing this.

I have however improved the error messages:

library(rsimsum)
library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
data("nlp", package = "rsimsum")
# estvarname:
rsimsum::simsum(
  data = rename(nlp, est = b), estvarname = "est", true = 0, se = "se",
  methodvar = "model", by = c("baseline", "ss", "esigma")
)
#> Error: 'est' is not an allowed name for 'estvarname'
# se:
rsimsum::simsum(
  data = rename(nlp, est = se), estvarname = "b", true = 0, se = "est",
  methodvar = "model", by = c("baseline", "ss", "esigma")
)
#> Error: 'est' is not an allowed name for 'se'
# methodvar:
rsimsum::simsum(
  data = rename(nlp, est = model), estvarname = "b", true = 0, se = "se",
  methodvar = "est", by = c("baseline", "ss", "esigma")
)
#> Error: 'est' is not an allowed name for 'methodvar'
# by:
rsimsum::simsum(
  data = rename(nlp, est = ss), estvarname = "b", true = 0, se = "se",
  methodvar = "model", by = c("baseline", "est", "esigma")
)
#> Error: 'est' is not an allowed name for 'by'

Created on 2023-05-27 with reprex v2.0.2

Do you think this is more clear?

I also aim to improve the package documentation to be more explicit about this requirement.

@ellessenne ellessenne added this to the 0.13.0 milestone Aug 26, 2023
@ellessenne
Copy link
Owner

This is fixed in {rsimsum} 0.13.0, which is now on CRAN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants