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

add_quantiles #288

Merged
merged 9 commits into from
Dec 27, 2021
Merged

add_quantiles #288

merged 9 commits into from
Dec 27, 2021

Conversation

timtreis
Copy link
Collaborator

@timtreis timtreis commented Nov 2, 2021

as discussed in #93

tagging @joanacmbarros since I can't tag her as a reviewer?

@timtreis timtreis self-assigned this Nov 2, 2021
@timtreis timtreis linked an issue Nov 2, 2021 that may be closed by this pull request
@timtreis timtreis added enhancement New feature or request discussion Discussion around design choices and architecture labels Nov 2, 2021
R/add_quantiles.R Outdated Show resolved Hide resolved
R/add_quantiles.R Outdated Show resolved Hide resolved
R/add_quantiles.R Outdated Show resolved Hide resolved
R/add_quantiles.R Outdated Show resolved Hide resolved
R/add_quantiles.R Show resolved Hide resolved
R/add_quantiles.R Outdated Show resolved Hide resolved
R/add_quantiles.R Outdated Show resolved Hide resolved
tests/testthat/test-add_quantiles.R Outdated Show resolved Hide resolved
tests/testthat/test-add_quantiles.R Outdated Show resolved Hide resolved
R/add_quantiles.R Outdated Show resolved Hide resolved
R/add_quantiles.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@SHAESEN2 SHAESEN2 left a comment

Choose a reason for hiding this comment

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

Fine for me but make sure that all testing is in place before merging.

@timtreis timtreis marked this pull request as ready for review November 18, 2021 16:33
R/add_quantiles.R Outdated Show resolved Hide resolved
R/visr.R Outdated Show resolved Hide resolved
R/visr.R Outdated Show resolved Hide resolved
R/visr.R Show resolved Hide resolved
R/visr.R Show resolved Hide resolved
R/visr.R Outdated Show resolved Hide resolved
tests/testthat/test-add_quantiles.R Outdated Show resolved Hide resolved
@timtreis timtreis requested a review from SHAESEN2 November 19, 2021 13:07
@timtreis
Copy link
Collaborator Author

Check is at 0/0/0, codecov at 100 %

R/add_quantiles.R Outdated Show resolved Hide resolved
R/visr.R Show resolved Hide resolved
tests/testthat/test-add_quantiles.R Outdated Show resolved Hide resolved
@SHAESEN2
Copy link
Collaborator

Nice work. Some details pending :)

@SHAESEN2 SHAESEN2 closed this Nov 20, 2021
@SHAESEN2 SHAESEN2 reopened this Nov 20, 2021
@timtreis timtreis requested a review from SHAESEN2 November 24, 2021 05:47
@timtreis timtreis changed the title add_quantiles - WIP add_quantiles Nov 24, 2021
- added more tests for visr survfit
- updated visR to align with expectations
- default method should not be a test. It should provide a default way of plotting objects
@SHAESEN2
Copy link
Collaborator

Hey @timtreis I have reviewed the branch. For me OK to merge now unless you have comments on my commit.

@timtreis timtreis merged commit 24572ce into develop Dec 27, 2021
@timtreis timtreis deleted the feature/quantile_lines branch December 27, 2021 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion around design choices and architecture enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KM plot show quantiles
2 participants