-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Testing - tern fix for analysis function refactor bugs #189
base: main
Are you sure you want to change the base?
Conversation
Unit Tests Summary 1 files 111 suites 3m 16s ⏱️ Results for commit 062247b. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit 0012b89 ♻️ This comment has been updated with latest results. |
The snapshot update is not related to tern - it comes from the data. I'm not sure when the data was changed, but the change to the LBT03 snapshot reflects the expected result as all pharmaverseadam::adlb |>
dplyr::filter(AVISIT == "Baseline") |>
dplyr::pull(CHG) |>
unique()
#> [1] NA Created on 2025-03-06 with reprex v2.1.1 |
n 86 84 82 | ||
Mean (SD) 17.57 (9.22) 19.20 (10.05) 17.96 (8.72) | ||
Median 15.00 16.00 17.00 | ||
Min - Max 7.00 - 69.00 6.00 - 64.00 5.00 - 70.00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to follow up on this, joining thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this happened before.
https://github.com/insightsengineering/scda.test/pull/161/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I think that was a but introduced here back then
https://github.com/insightsengineering/tern/pull/1347/files
this is really odd. lets follow up, @edelarua , I was wondeing when updating the snapshot, are you updating it locally on your machine? |
Yes, it happens locally and in the integration tests. The updated results make sense since the change from baseline values at the baseline visit should be 0/NA, not what we had previously. |
I would like to check the computation in tern again, perhaps we caught a bug that was not found before. The data change happened for some time already, but we have been refactoring the computation. |
See insightsengineering/tern#1406