Last obs before exposure flag and baseline flag #49

kamilsi · 2024-05-08T15:29:11Z

@rammprasad, please verify the functionality of the new code additions. The existing examples confirm basic operations, but I was unable to fully test the baseline visit logic due to unmarked baseline visits in the study configuration file. Your insights or additional test cases would be helpful.

…amples

github-actions · 2024-05-08T15:35:09Z

Package	Line Rate	Health
sdtm.oak	88%	✔
Summary	88% (901 / 1023)	✔

ramiromagno · 2024-05-14T23:17:15Z

Hi @rammprasad: @kamilsi asked for my review too but I noticed you pushed a few commits just an hour ago. Do you intend to make further changes until tomorrow's meeting or may look into it?

rammprasad · 2024-05-15T05:08:08Z

Hi @rammprasad: @kamilsi asked for my review too but I noticed you pushed a few commits just an hour ago. Do you intend to make further changes until tomorrow's meeting or may look into it?

This is not ready for review. I have requested Kamil to look at my changes, replace the assertions, and add a test case. We will try and send it out for review tomorrow.

…mple + validations

rammprasad

Working as expected. REady for second review.

Needs more work

rammprasad · 2024-06-11T20:50:54Z

@kamilsi - this is working as expected. I made minor tweaks. Can you resolve the merge conflicts and prepare it for a second review and merge?

Merge branch 'main' into 18-last-obs-before-exposure-flag-and-baseline-flag # Conflicts: # NAMESPACE

R/derive_blfl.R

tests/testthat/test-derive_blfl.R

R/derive_blfl.R

man/derive_blfl.Rd

R/derive_blfl.R

rammprasad · 2024-06-19T19:27:23Z

I have fixed a few minor items. Once this PR is approved, we can address the issues below.

Refactor code based on suggestions - Last obs before exposure flag and baseline flag #49 (comment) and Last obs before exposure flag and baseline flag #49 (comment)
Validate the ISO date formats.
Add test case for baseline_timepoints functionality

rammprasad · 2024-06-19T19:31:48Z

@ShiyuC @ramiromagno - Ready for your approval

ramiromagno · 2024-06-19T19:36:51Z

I'm getting this check message about potential spelling errors:

   < Potential spelling errors:
   <   WORD        FOUND IN
   < RFSTDTC     derive_blfl.Rd:27
   < RFXSTDTC    derive_blfl.Rd:28
   < TPT         derive_blfl.Rd:35
   < pre         derive_blfl.Rd:77
   < timpoints   derive_blfl.Rd:35
   < varaible    derive_blfl.Rd:19
   < xxTPT       derive_blfl.Rd:73
   < If these are false positive, run `spelling::update_wordlist()`.All Done!

ShiyuC

Thanks Ram! I’m ooo this afternoon but I’m happy to give the approval now as I saw my comments are resolved. If another review is needed, I’ll come back this evening to do it.

edgar-manukyan · 2024-06-20T16:06:48Z

I'm getting this check message about potential spelling errors:

   < Potential spelling errors:
   <   WORD        FOUND IN
   < RFSTDTC     derive_blfl.Rd:27
   < RFXSTDTC    derive_blfl.Rd:28
   < TPT         derive_blfl.Rd:35
   < pre         derive_blfl.Rd:77
   < timpoints   derive_blfl.Rd:35
   < varaible    derive_blfl.Rd:19
   < xxTPT       derive_blfl.Rd:73
   < If these are false positive, run `spelling::update_wordlist()`.All Done!

I will address this.

We now have even more

> spelling::spell_check_package()
  WORD         FOUND IN
addtional    events_domain.Rmd:274
appicable    findings_domain.Rmd:458
CMGRPID      cnd_df.Rmd:49
             events_domain.Rmd:191
CMMODIFY     events_domain.Rmd:358
CMSTRTPT     events_domain.Rmd:247
CMSTTPT      events_domain.Rmd:278
CMTRT        cnd_df.Rmd:49
             events_domain.Rmd:191
CRF          events_domain.Rmd:90
datetime     algorithms.Rmd:38
             events_domain.Rmd:60,219
devleoped    findings_domain.Rmd:559
DIABP        findings_domain.Rmd:201
eCRF         algorithms.Rmd:15
             events_domain.Rmd:88
             study_sdtm_spec.Rmd:44,55,58,58,61
eDT          algorithms.Rmd:15
             events_domain.Rmd:88
             study_sdtm_spec.Rmd:44,55,71,73
hardcode     algorithms.Rmd:38
             events_domain.Rmd:61,62,245,276
hardcoding   events_domain.Rmd:247,278
MDBDR        events_domain.Rmd:29
MDBTM        events_domain.Rmd:29
MDEDR        events_domain.Rmd:29
MDETM        events_domain.Rmd:29
MDR          algorithms.Rmd:24
             study_sdtm_spec.Rmd:40
MDRAW        events_domain.Rmd:29
mmm          events_domain.Rmd:221
NonCRF       events_domain.Rmd:90
OID          events_domain.Rmd:51
             findings_domain.Rmd:33
OXYSAT       findings_domain.Rmd:201
px           algorithms.Rmd:34,218
SYS          findings_domain.Rmd:118
SYSBP        findings_domain.Rmd:118,147
VSALL        findings_domain.Rmd:201
yyyy         events_domain.Rmd:221

ramiromagno · 2024-06-20T16:14:48Z

Edgar: I was getting that too earlier but no longer with the most recent version of the code. Have you pulled meanwhile?

…

On Thu, 20 Jun 2024, 17:07 edgar-manukyan, ***@***.***> wrote: I'm getting this check message about potential spelling errors: < Potential spelling errors: < WORD FOUND IN < RFSTDTC derive_blfl.Rd:27 < RFXSTDTC derive_blfl.Rd:28 < TPT derive_blfl.Rd:35 < pre derive_blfl.Rd:77 < timpoints derive_blfl.Rd:35 < varaible derive_blfl.Rd:19 < xxTPT derive_blfl.Rd:73 < If these are false positive, run `spelling::update_wordlist()`.All Done! I will address this. — Reply to this email directly, view it on GitHub <#49 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEXRG4DVVRP6M5Y7242CIATZIL425AVCNFSM6AAAAABHNGLO36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBRGA2TQMRUHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

edgar-manukyan · 2024-06-20T16:16:56Z

@ramiromagno, correct, I pulled and I got even more spelling mistakes, will push shortly.

edgar-manukyan · 2024-06-20T16:22:38Z

@ramiromagno, sorry for confusion, I was fixing in the 51-documentation-updates-v01

edgar-manukyan · 2024-06-20T16:26:09Z

Now I switched to 18-last-obs-before-exposure-flag-and-baseline-flag, pulled and I see the following and will push shortly

> spelling::spell_check_package()
  WORD        FOUND IN
CMGRPID     cnd_df.Rmd:49
CMTRT       cnd_df.Rmd:49
eCRF        algorithms.Rmd:15
            study_sdtm_spec.Rmd:44,55,58,58,61
eDT         algorithms.Rmd:15
            study_sdtm_spec.Rmd:44,55,71,73
MDR         algorithms.Rmd:24
            study_sdtm_spec.Rmd:40
pre         derive_blfl.Rd:77
            algorithms.Rmd:24
px          algorithms.Rmd:34,211
RFSTDTC     derive_blfl.Rd:27
RFXSTDTC    derive_blfl.Rd:28
timpoints   derive_blfl.Rd:35
TPT         derive_blfl.Rd:35
varaible    derive_blfl.Rd:19
xxTPT       derive_blfl.Rd:73

ramiromagno · 2024-06-20T16:31:19Z

Have you pulled? :) because those spelling issues should have been fixed by now...

…

On Thu, 20 Jun 2024, 17:26 edgar-manukyan, ***@***.***> wrote: Now I switched to 18-last-obs-before-exposure-flag-and-baseline-flag and I see the following and will push shortly > spelling::spell_check_package() WORD FOUND IN CMGRPID cnd_df.Rmd:49 CMTRT cnd_df.Rmd:49 eCRF algorithms.Rmd:15 study_sdtm_spec.Rmd:44,55,58,58,61 eDT algorithms.Rmd:15 study_sdtm_spec.Rmd:44,55,71,73 MDR algorithms.Rmd:24 study_sdtm_spec.Rmd:40 pre derive_blfl.Rd:77 algorithms.Rmd:24 px algorithms.Rmd:34,211 RFSTDTC derive_blfl.Rd:27 RFXSTDTC derive_blfl.Rd:28 timpoints derive_blfl.Rd:35 TPT derive_blfl.Rd:35 varaible derive_blfl.Rd:19 xxTPT derive_blfl.Rd:73 — Reply to this email directly, view it on GitHub <#49 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEXRG4FT3EGGQBWGJ5ZOGO3ZIL7DPAVCNFSM6AAAAABHNGLO36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBRGA4TGMRTGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

edgar-manukyan · 2024-06-20T16:31:37Z

@ramiromagno, the spellings should be good now. Can you please approve?

edgar-manukyan · 2024-06-20T16:34:23Z

Have you pulled? :) because those spelling issues should have been fixed by now...

I pulled and there were few findings, I fixed those and added the words from the #56 as well.

ramiromagno

All good.

edgar-manukyan · 2024-06-20T17:01:13Z

I will fix the conflicts

…e-flag

kamilsi added 2 commits April 11, 2024 10:32

WIP: Baseline flag code adaptation, not functional yet

a70fc30

WIP: working code, pending tests for baseline logic due to lack of ex…

dc2c732

…amples

kamilsi requested review from ramiromagno and rammprasad May 8, 2024 15:29

kamilsi linked an issue May 8, 2024 that may be closed by this pull request

Last obs before exposure flag and Baseline Flag #18

Closed

rammprasad added 3 commits May 14, 2024 22:11

remove files as added in examples

491252e

modified function

675fcef

Harmonize input parameters with other derive_studyday function

b4fa080

ramiromagno assigned kamilsi May 14, 2024

kamilsi and others added 4 commits May 20, 2024 20:54

Switched from checkmate to admiraldev assertions; added tests for exa…

c0f90b9

…mple + validations

styler

bcc8a37

lintr fixes

3456c52

Fixed baseline visit feature and added a test case

c92ae50

rammprasad previously approved these changes Jun 11, 2024

View reviewed changes

rammprasad self-requested a review June 11, 2024 20:49

merge main to FB

a4143de

Merge branch 'main' into 18-last-obs-before-exposure-flag-and-baseline-flag # Conflicts: # NAMESPACE

rammprasad marked this pull request as ready for review June 18, 2024 05:52

rammprasad added 7 commits June 18, 2024 05:58

Run Styler

a4fa80b

Fix pipeline failures

a86e4a9

Fix pipeline failure

3e9782e

Fix style

6054a59

Fix pipeline failure

d9d2e5e

fix pipeline failures

2b1a9c0

Fix pipeline failures

1332197

ramiromagno reviewed Jun 18, 2024

View reviewed changes

R/derive_blfl.R Show resolved Hide resolved

ShiyuC reviewed Jun 19, 2024

View reviewed changes

tests/testthat/test-derive_blfl.R Show resolved Hide resolved

tests/testthat/test-derive_blfl.R Show resolved Hide resolved

tests/testthat/test-derive_blfl.R Show resolved Hide resolved

tests/testthat/test-derive_blfl.R Show resolved Hide resolved

R/derive_blfl.R Show resolved Hide resolved

ShiyuC reviewed Jun 19, 2024

View reviewed changes

R/derive_blfl.R Show resolved Hide resolved

ramiromagno reviewed Jun 19, 2024

View reviewed changes

man/derive_blfl.Rd Outdated Show resolved Hide resolved

ramiromagno reviewed Jun 19, 2024

View reviewed changes

R/derive_blfl.R Show resolved Hide resolved

rammprasad added 2 commits June 19, 2024 18:59

Review comments

d88093b

Fix errors

cb3b9cb

rammprasad mentioned this pull request Jun 19, 2024

derive_blfl - Address review comments #59

Open

ramiromagno requested review from ramiromagno, rammprasad and ShiyuC June 19, 2024 19:38

ShiyuC approved these changes Jun 19, 2024

View reviewed changes

edgar-manukyan closed this Jun 20, 2024

edgar-manukyan reopened this Jun 20, 2024

fix spelling

ff2d777

ramiromagno approved these changes Jun 20, 2024

View reviewed changes

Merge branch 'main' into 18-last-obs-before-exposure-flag-and-baselin…

49cfee2

…e-flag

rammprasad merged commit 6abb5ba into main Jun 26, 2024
13 checks passed

rammprasad deleted the 18-last-obs-before-exposure-flag-and-baseline-flag branch June 26, 2024 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Last obs before exposure flag and baseline flag #49

Last obs before exposure flag and baseline flag #49

kamilsi commented May 8, 2024

github-actions bot commented May 8, 2024 •

edited

Loading

ramiromagno commented May 14, 2024

rammprasad commented May 15, 2024

rammprasad left a comment

rammprasad commented Jun 11, 2024

rammprasad commented Jun 19, 2024 •

edited

Loading

rammprasad commented Jun 19, 2024

ramiromagno commented Jun 19, 2024

ShiyuC left a comment

edgar-manukyan commented Jun 20, 2024 •

edited

Loading

ramiromagno commented Jun 20, 2024 via email

edgar-manukyan commented Jun 20, 2024

edgar-manukyan commented Jun 20, 2024

edgar-manukyan commented Jun 20, 2024 •

edited

Loading

ramiromagno commented Jun 20, 2024 via email

edgar-manukyan commented Jun 20, 2024

edgar-manukyan commented Jun 20, 2024 •

edited

Loading

ramiromagno left a comment

edgar-manukyan commented Jun 20, 2024 •

edited

Loading

Last obs before exposure flag and baseline flag #49

Last obs before exposure flag and baseline flag #49

Conversation

kamilsi commented May 8, 2024

github-actions bot commented May 8, 2024 • edited Loading

ramiromagno commented May 14, 2024

rammprasad commented May 15, 2024

rammprasad left a comment

Choose a reason for hiding this comment

rammprasad commented Jun 11, 2024

rammprasad commented Jun 19, 2024 • edited Loading

rammprasad commented Jun 19, 2024

ramiromagno commented Jun 19, 2024

ShiyuC left a comment

Choose a reason for hiding this comment

edgar-manukyan commented Jun 20, 2024 • edited Loading

ramiromagno commented Jun 20, 2024 via email

edgar-manukyan commented Jun 20, 2024

edgar-manukyan commented Jun 20, 2024

edgar-manukyan commented Jun 20, 2024 • edited Loading

ramiromagno commented Jun 20, 2024 via email

edgar-manukyan commented Jun 20, 2024

edgar-manukyan commented Jun 20, 2024 • edited Loading

ramiromagno left a comment

Choose a reason for hiding this comment

edgar-manukyan commented Jun 20, 2024 • edited Loading

github-actions bot commented May 8, 2024 •

edited

Loading

rammprasad commented Jun 19, 2024 •

edited

Loading

edgar-manukyan commented Jun 20, 2024 •

edited

Loading

edgar-manukyan commented Jun 20, 2024 •

edited

Loading

edgar-manukyan commented Jun 20, 2024 •

edited

Loading

edgar-manukyan commented Jun 20, 2024 •

edited

Loading