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

Add supp tbl #38

Merged
merged 8 commits into from
Mar 10, 2022
Merged

Add supp tbl #38

merged 8 commits into from
Mar 10, 2022

Conversation

statasaurus
Copy link
Collaborator

No description provided.

@statasaurus statasaurus requested a review from mstackhouse March 10, 2022 10:08
@codecov
Copy link

codecov bot commented Mar 10, 2022

Codecov Report

Merging #38 (bf6ab06) into dev (56f4c88) will decrease coverage by 1.01%.
The diff coverage is 73.52%.

❗ Current head bf6ab06 differs from pull request most recent head a35b8f4. Consider uploading reports for the commit a35b8f4 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #38      +/-   ##
==========================================
- Coverage   85.30%   84.29%   -1.02%     
==========================================
  Files           6        6              
  Lines         871      923      +52     
==========================================
+ Hits          743      778      +35     
- Misses        128      145      +17     
Impacted Files Coverage Δ
R/validators.R 85.05% <56.25%> (-6.73%) ⬇️
R/metacore.R 90.44% <88.88%> (-0.46%) ⬇️

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 56f4c88...a35b8f4. Read the comment docs.

@statasaurus statasaurus merged commit b21f965 into dev Mar 10, 2022
@statasaurus statasaurus deleted the add_supp_tbl branch March 10, 2022 11:30
codes = "List of Codes")

private$.supp <- supp %>%
add_labs(dataset = "Dataset Name",
Copy link
Contributor

Choose a reason for hiding this comment

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

@statasaurus is the assumption that the variable would also live in var_spec and flagged as a supp? And QORIG based on derivations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No the QORIG would be base on the origin. You shouldn't need to have a supp flag in var_spec or value_spec cause the information will be pulled from ds_vars. The plan would be you would treat your supp variable like any other. Add the label to the label caloumn in var_spec, add information about how it is derived in derivations and any origin, codelist etc. information in the value_spec

Copy link
Contributor

Choose a reason for hiding this comment

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

Before we polish this off, could you put together an example to merge the necessary info from valtools and push into the supp building functions? Before we call this final I just want to make sure that we have that metcaore -> metatools -> supp dataset workflow locked down.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

... valtools? Did you mean metatools? The example is here. I have created a mock metacore sdtm object which is already in the dev branch that has some supp stuff it in. I am planning on writing a where to add my supp information vingette.

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