Last obs before exposure flag and baseline flag

kamilsi commented 2 months ago

@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.

github-actions[bot] commented 2 months ago

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

ramiromagno commented 2 months ago

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 commented 2 months ago

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.

rammprasad commented 1 month ago

@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?

rammprasad commented 4 weeks ago

@ShiyuC @ramiromagno - Ready for your review.

rammprasad commented 4 weeks ago

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

Refactor code based on suggestions - https://github.com/pharmaverse/sdtm.oak/pull/49#discussion_r1645492545 and https://github.com/pharmaverse/sdtm.oak/pull/49#discussion_r1645420117
Validate the ISO date formats.
Add test case for baseline_timepoints functionality

rammprasad commented 4 weeks ago

@ShiyuC @ramiromagno - Ready for your approval

ramiromagno commented 4 weeks ago

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!

edgar-manukyan commented 3 weeks ago

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 commented 3 weeks ago

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 https://github.com/pharmaverse/sdtm.oak/pull/49#issuecomment-2181058249, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXRG4DVVRP6M5Y7242CIATZIL425AVCNFSM6AAAAABHNGLO36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBRGA2TQMRUHE . You are receiving this because you were mentioned.Message ID: @.***>

edgar-manukyan commented 3 weeks ago

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

edgar-manukyan commented 3 weeks ago

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

edgar-manukyan commented 3 weeks ago

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 commented 3 weeks ago

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 https://github.com/pharmaverse/sdtm.oak/pull/49#issuecomment-2181093234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXRG4FT3EGGQBWGJ5ZOGO3ZIL7DPAVCNFSM6AAAAABHNGLO36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBRGA4TGMRTGQ . You are receiving this because you were mentioned.Message ID: @.***>

edgar-manukyan commented 3 weeks ago

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

edgar-manukyan commented 3 weeks ago

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 https://github.com/pharmaverse/sdtm.oak/pull/56 as well.

edgar-manukyan commented 3 weeks ago

I will fix the conflicts

pharmaverse / sdtm.oak

Last obs before exposure flag and baseline flag #49