UW-GAC / dbgaptools

R package to create and check standard files for dbGaP submission
Other
1 stars 3 forks source link

adding "good" dd to check_sattr() and check_pheno() causes testing error #39

Open broomej opened 4 years ago

broomej commented 4 years ago

I added a test expect_null(check_sattr(dsfile=sattr_ds, ddfile=sattr_dd)) to testthat, and adding what should be a passing dd dictionary causes the test to fail.

I can look at this after #30, but it should maybe wait since there are some formatting changes. I added a commit/branch here: 0936605c9da3ccd290e67eb09975f38971b6ab7e

broomej commented 4 years ago

I also noticed a similar problem adding the following test for check_pheno():

    expect_null(check_pheno(dsfile = pheno_ds, ddfile = pheno_dd,
                            na_vals = c("NA", "N/A", "na", "n/a", "9999")))
broomej commented 4 years ago

This is because of the unexpected variable in the sattr DD VARIABLE_TERM. Are we considering that an allowed column in the DD?

broomej commented 4 years ago

I also merged in #40 to begin work on this

broomej commented 4 years ago

VARIABLE_TERM is not mentioned in the submission guide appendix, so I'm going to consider it an illegal field

broomej commented 4 years ago

The problem with the pheno dictionary is a little trickier. Run check_pheno(ds = pheno_ds, dd = pheno_dd, na_vals = c("NA", "N/A", "na", "n/a", "9999")) in test_check_pheno.R to see the output

broomej commented 4 years ago

The check when running check_sattr() is passing when dataframes are passed as arguments because I dropped the illegal variable name when reading it in (see b1cc1231edf56a2cb9d33ecf06bfe958e0a01044). It does not work when passing the path to the file on disk because that version of the file still has that illegal variable.