Skip to content

Derive a replacement variable#162

Open
nealrichardson wants to merge 1 commit intomasterfrom
replace-variable
Open

Derive a replacement variable#162
nealrichardson wants to merge 1 commit intomasterfrom
replace-variable

Conversation

@nealrichardson
Copy link
Contributor

Sometimes when you combine categories or do some other derivation, you don't want to see the input variable anymore. But it's useful to keep it around so that if more data is appended, the derived variable updates.

This PR creates a means of attaching metadata to VariableDefinitions when creating them that indicates that, when they are added to the dataset, that some other variable(s) should be hidden and this new variable should appear in its place in the order.

This should also resolve/standardize some of the awkwardness in #154

@codecov
Copy link

codecov bot commented Nov 15, 2017

Codecov Report

Merging #162 into master will decrease coverage by 0.25%.
The diff coverage is 15.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
- Coverage   89.21%   88.95%   -0.26%     
==========================================
  Files          92       92              
  Lines        5479     5497      +18     
==========================================
+ Hits         4888     4890       +2     
- Misses        591      607      +16
Impacted Files Coverage Δ
R/add-variable.R 75.28% <6.25%> (-15.26%) ⬇️
R/combine-categories.R 97.66% <66.66%> (-0.56%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54fcd7c...0c2a098. Read the comment docs.

@nealrichardson nealrichardson mentioned this pull request Nov 16, 2017
Copy link
Contributor

@jonkeane jonkeane left a comment

Choose a reason for hiding this comment

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

The code looks good to me. I'm a little bit hesitant that replace might sound misleading to the user as being a literal replace the variable (especially with respect to the alias). I can see that making the replacement variable have the same alias as the original could break things like streaming and appends. So this is probably as good as we can get, and really as good as people need. But maybe a note saying that replace=TRUE will and must have a new alias that is unique.

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.

2 participants