Skip to content
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.

Sign up for GitHub

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

Jump to bottom

51 documentation updates v01 #56

Merged
merged 39 commits into from
Jun 20, 2024
Merged

51 documentation updates v01 #56

merged 39 commits into from
Jun 20, 2024

Conversation

edgar-manukyan
Copy link
Collaborator

Thank you for your Pull Request! We have developed this task checklist from the
Development Process
Guide
to help with the final steps of the process. Completing the below tasks helps to
ensure our reviewers can maximize their time on your code as well as making sure
the oak codebase remains robust and consistent.

Please check off each taskbox as an acknowledgment that you completed the task
or check off that it is not relevant to your Pull Request. This checklist is
part of the Github Action workflows and the Pull Request will not be merged into
the devel branch until you have checked off each task.

  • Place Closes #<insert_issue_number> into the beginning of your Pull z
    Request Title (Use Edit button in top-right if you need to update)
  • Code is formatted according to the
    tidyverse style guide. Run
    styler::style_file() to style R and Rmd files
  • Updated relevant unit tests or have written new unit tests, which should
    consider realistic data scenarios and edge cases, e.g. empty datasets, errors,
    boundary cases etc. - See
    Unit Test Guide
  • If you removed/replaced any function and/or function parameters, did you
    fully follow the
    deprecation guidance?
  • Update to all relevant roxygen headers and examples, including keywords
    and families. Refer to the
    categorization of functions to tag appropriate keyword/family.
  • Run devtools::document() so all .Rd files in the man folder and the
    NAMESPACE file in the project root are updated appropriately
  • Address any updates needed for vignettes and/or templates
  • Update NEWS.md if the changes pertain to a user-facing function (i.e. it
    has an @export tag) or documentation aimed at users (rather than developers)
  • Build oak site pkgdown::build_site() and check that all affected
    examples are displayed correctly and that all new functions occur on the "Reference" page.
  • Address or fix all lintr warnings and errors - lintr::lint_package()
  • Run R CMD check locally and address all errors and warnings - devtools::check()
  • Link the issue in the Development Section on the right hand side.
  • Address all merge conflicts and resolve appropriately
  • Pat yourself on the back for a job well done! Much love to your accomplishment!

rammprasad and others added 22 commits May 15, 2024 05:08
@rammprasad
New function to derive oak_id_vars. More work needed.
0300e23
@rammprasad
cm template updates. In progress.
b99fb5d
@rammprasad
almost completed cm template
0d56384
@ramiromagno
Basic support for "conditioned" data frames
8dfa57f 
- Adds a new S3 class (cnd_df) for represented conditioned data frames, i.e. data frames that carry metadata about what records should be used for derivations

- Adds support for basic pretty printing of cnd_df objects

- Adds a user-facing function for creating such cnd_df objects: `condition_by`

- Adds experimental "mutate"-version function for these conditioned data frames: `derive_by_condition()`
@ramiromagno
Basic support for conditioned data sets
d08794b
@ramiromagno
Extensive support for conditioned tibbles
00e758a 
- Joins by raw and target data sets are now aware of conditioned tibbles
- Transformation functions, namely `assign_datetime()`, `hardcode*()` and `assign*` are also conditioned-tibble aware
- Unit test coverage for most cases indicated at #54

I believe the essential components are here to support the if_then_else algorithm via conditioned tibbles. Now, further testing, assertions and documentation is needed.
@rammprasad
Merge branch '0054-condition-by' into 51-documentation-updates-v01 to…
783f653 
… help with template code

Merge branch '0054-condition-by' into 51-documentation-updates-v01
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@ramiromagno
Ramm's feedback integration
0d7861a 
- Move `tgt_dat` to the first position in the argument list for cleaner command pipes.

- Rename `condition_by()` to `condition_add()`.

- Export `oak_id_vars()` for direct user access.

- Update tidyselections to align with the latest practices.
@rammprasad
A fix to derive study day
eea1580
@rammprasad
Algorithms Vignette update
1b4c849
@rammprasad
cm template code update
9b5d9b4
@rammprasad
Merge branch '0054-condition-by' into FB to help with programming
8330232 
Merge branch '0054-condition-by' into 51-documentation-updates-v01
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@rammprasad
A function to help display of dataset in Vignette
99ea428
@rammprasad
Template update
128d4dc
@rammprasad
Raw data change
754d6c7
@rammprasad
DM domain csv
c847935
@rammprasad
Events domain article
503c145
@rammprasad
update controlled terminology
2b134f6
@rammprasad
Updated CM template
6266fe0
@rammprasad
VS domain template and Vignette
7ae3275
@rammprasad
CM domain Vignette update
48ed544
@ramiromagno
Update on conditioned data frames
fd722ba 
- Documentation
- Examples
- New article about cnd_df (WIP)
@edgar-manukyan edgar-manukyan linked an issue Jun 17, 2024 that may be closed by this pull request
Documentation updates V0.1 #51
Closed
@rammprasad
Fix Vignette
9e66fad
@rammprasad
merge to FB for documentation.
ef7e86c 
Merge branch '0054-condition-by' into 51-documentation-updates-v01
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@rammprasad
Updates to code
c5aea02
@rammprasad
merge main to FB and resolve conflicts
af29fa7 
Merge branch 'main' into 51-documentation-updates-v01

# Conflicts:
#	NAMESPACE
#	R/assign.R
#	R/assign_datetime.R
#	R/cnd_df.R
#	R/join.R
#	R/pipe.R
#	R/sdtm_join.R
#	_pkgdown.yml
#	inst/WORDLIST
#	man/condition_add.Rd
#	man/dot_pipe.Rd
#	man/mutate.cnd_df.Rd
#	man/tbl_sum.cnd_df.Rd
#	tests/testthat/test-assign.R
#	tests/testthat/test-cnd_df.R
#	tests/testthat/test-condition_add.R
#	tests/testthat/test-eval_conditions.R
#	tests/testthat/test-hardcode.R
#	tests/testthat/test-mutate_cnd_df.R
#	tests/testthat/test-pipe.R
#	vignettes/articles/cnd_df.Rmd
@rammprasad
Remove white spaces
fd1f60f
@rammprasad
remove white spaces
a0bab7f
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 19, 2024 •
edited
Loading

Code Coverage

Package Line Rate Health
sdtm.oak 88% ✔
Summary 88% (786 / 894) ✔

@rammprasad
Copy link
Collaborator

@edgar-manukyan @galachad @ramiromagno - I think all checks passed in the pipeline. But it is not showing up. Can you help me resolve this? From a content perspective, everything is complete.

@ramiromagno
Copy link
Collaborator

The DT package is in Suggests but should be in Imports because it's used in dataset_oak_vignette().

@ramiromagno
Copy link
Collaborator

Also, with R version 4.4 and more recent packages, I am also getting:

Using an external vector in selections was deprecated in tidyselect 1.1.0.
i Please use `all_of()` or `any_of()` instead.
  # Was:
  data %>% select(.after)

  # Now:
  data %>% select(all_of(.after))

These warnings come from the functions implemented in the branch with conditioned data frames, and I know I have fixed them before we merged into main. So I guess this branch does not yet have the most up-to-date version of main integrated into it...?

@rammprasad
Copy link
Collaborator

I have merged the latest main to this branch. Not sure what is going on. It doesn't show any differences.

ramiromagno
ramiromagno approved these changes Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@ramiromagno ramiromagno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rammprasad All good. I just added one commit to update the WORDLIST.

@edgar-manukyan edgar-manukyan mentioned this pull request Jun 20, 2024
Merged
@ramiromagno ramiromagno self-requested a review June 20, 2024 16:39
ramiromagno
ramiromagno approved these changes Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@ramiromagno ramiromagno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good.

@rammprasad rammprasad merged commit 08cc5de into main Jun 20, 2024
14 of 15 checks passed
@rammprasad rammprasad deleted the 51-documentation-updates-v01 branch June 20, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rammprasad rammprasad rammprasad approved these changes

@ramiromagno ramiromagno ramiromagno approved these changes

@yli110-stat697 yli110-stat697 Awaiting requested review from yli110-stat697

Assignees

@rammprasad rammprasad

Labels
None yet
Projects
sdtm.oak R package
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Documentation updates V0.1
3 participants
@edgar-manukyan @rammprasad @ramiromagno