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.

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

Gdr 2419 #140

Merged
merged 22 commits into from
Feb 28, 2024
Merged

Gdr 2419 #140

merged 22 commits into from
Feb 28, 2024

Conversation

gladkia
Copy link
Contributor

@gladkia gladkia commented Feb 26, 2024

Description

Improve pkgdown site

What changed?

Logistic checklist

  • Package version bumped
  • Changelog updated

Screenshots (optional)

@gladkia gladkia requested a review from a team as a code owner February 26, 2024 11:51
@gladkia gladkia requested review from darsoo and bczech and removed request for a team February 26, 2024 11:51
Copy link
Contributor

@j-smola j-smola left a comment

Choose a reason for hiding this comment

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

Could you maybe rethink group for functions?

  1. IMO not all function in "Combination helpers" is used only for combination data.
  2. "Mappping" seems to be list of function supporting "Pipeline"
  3. "Utils" and "Preparing input" are both "Preparing input for pipline"? As also "Identifying keys", right?
  4. etc.

Please, consider also

  1. update or "gDRcore.Rmd" vignette - "Drug processing section" section where due to lack of empty line list of steps disappeared;
  2. update the gDRcore architecture diagram in READMA.md as it is no longer readable on the pkgdown site.

PS: And fot is the reasone for function from constant.R file?

NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
gladkia and others added 6 commits February 26, 2024 17:29
Co-authored-by: j-smola <[email protected]>
Co-authored-by: j-smola <[email protected]>
Co-authored-by: j-smola <[email protected]>
Co-authored-by: j-smola <[email protected]>
Co-authored-by: j-smola <[email protected]>
Co-authored-by: j-smola <[email protected]>
@gladkia
Copy link
Contributor Author

gladkia commented Feb 26, 2024

Could you maybe rethink group for functions?

  1. IMO not all function in "Combination helpers" is used only for combination data.
  2. "Mappping" seems to be list of function supporting "Pipeline"
  3. "Utils" and "Preparing input" are both "Preparing input for pipline"? As also "Identifying keys", right?
  4. etc.

Please, consider also

  1. update or "gDRcore.Rmd" vignette - "Drug processing section" section where due to lack of empty line list of steps disappeared;
  2. update the gDRcore architecture diagram in READMA.md as it is no longer readable on the pkgdown site.

PS: And fot is the reasone for function from constant.R file?

1st section

  1. Thanks. Fixed.
  2. Yes. Please note that runDrugResponseProcessingPipeline() is the top-level function for running the whole pipeline, dependent on multiple functions from gDRcore.
  3. Prepare input refers to the S3 methods called 'prepare_input' for data.table and MAE as well as their dependencies (internal helper functions). The utils comprises a broader, less specific category I've moved a single function from 'Identifying keys' to the utils category.

2nd section

  1. Done.
  2. Adrian suggested getting rid of this diagram from README.md. I'll try to prepare an analogous diagram with plantuml. It will be added to the vignette instead of the README.md.
  3. 'constants.R' - there are no functions there - only three constant objects: drugNameRegex, untreated_tag_patterns, and untreatedDrugNameRegex.

@gladkia
Copy link
Contributor Author

gladkia commented Feb 28, 2024

I've just added pipeline diagrams to Rmd. See 4b6f3d7 and 0c74a03.
@bczech , @j-smola FYI.

README.md Outdated Show resolved Hide resolved
vignettes/gDRcore.Rmd Show resolved Hide resolved
@gladkia gladkia merged commit 0ef5395 into main Feb 28, 2024
3 of 4 checks passed
@gladkia gladkia deleted the GDR-2419 branch February 28, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants