length 12 for PPSTRESN #591
Conversation
Gero1999
left a comment
Gero1999
left a comment
There was a problem hiding this comment.
@Shaakon35 I run on P21 and found still the issues
| # Parameter Variables | ||
| PPORRES = as.character(round(as.numeric(PPORRES), 12)), | ||
| PPSTRESN = round(as.numeric(PPSTRES), 12), | ||
| PPSTRESC = as.character(format(PPSTRESN, scientific = FALSE, trim = TRUE)), |
There was a problem hiding this comment.
Consider not doing this
issues:
- PPSTRESN does not still match PPSTRESC in the conflictive row (
XX01-25103, AUCPEP, Analyte01) - doing this there is a problem with the scientific nomenclature if you do not force it to FALSE...
There was a problem hiding this comment.
If I don't do this, I have an issue with PPSTRESC decimal values = 0000 at the end
R/export_cdisc.R
Outdated
| } else if (var_specs$Type %in% c("Num", "integer", "float") && | ||
| !endsWith(var, "DTM")) { | ||
| df[[var]] <- round(as.numeric(df[[var]]), var_specs$Length) | ||
| df[[var]] <- round(as.numeric(df[[var]]), 12) |
There was a problem hiding this comment.
as mentioned I think the problem is the round() operation, not the number itself, no need to do it, it can be the reason of the differences PPSTRESN, PPSTRESC
|
I think that still is not a problem in the XPT submission file, but as this is not priority I will let this PR breathe and think about it |
|
All good !
|
…resc-secondoption
some things has changed, including: all numerics -> length = 12 PPGRPID -> length = 100
…ub.com/pharmaverse/aNCA into 561-bug-ppstresn-ppstresc-secondoption
|
@Gero1999 Ready to review. All good, now PP passed Pinnacle 21 checks, with two NCA_PROFILE selected. |
|
@Gero1999 Waiting for review |
m-kolomanski
left a comment
m-kolomanski
left a comment
There was a problem hiding this comment.
Code-wise looks good, just a minor suggestion
| PPORRES = as.numeric( | ||
| substr( | ||
| as.numeric(round(PPORRES, 8)), | ||
| 0, | ||
| 8 + grepl("\\.", as.character(PPORRES)) | ||
| ) | ||
| ), | ||
| PPSTRESN = as.numeric( | ||
| substr( | ||
| as.numeric(round(PPSTRES, 8)), | ||
| 0, | ||
| 8 + grepl("\\.", as.character(PPSTRES)) | ||
| ) | ||
| ), |
There was a problem hiding this comment.
suggestion: This probably should be enclosed in a separate helper function, as the logic is repeated.
There was a problem hiding this comment.
you are right, this is my bad. I can take care of it during my review @Shaakon35
|
Hey @Shaakon35 still having the issues with PPSTRESN != PPSTRESC unfortunately 😓 |
Issue
Closes #561
How to test
Now, you can try to submit PP in Pinnacle 21 and it should work!
Contributor checklist