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 sample size and events to sim_gs_n per analysis per simulation #208

Merged
merged 3 commits into from
Feb 27, 2024

Conversation

LittleBeannie
Copy link
Collaborator

@LittleBeannie LittleBeannie commented Feb 27, 2024

Closes #207

@LittleBeannie LittleBeannie linked an issue Feb 27, 2024 that may be closed by this pull request
Copy link
Collaborator

@nanxstats nanxstats left a comment

Choose a reason for hiding this comment

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

Instead of removing n and event from observed, I think the better way to update the tests is to add these to expected.

@LittleBeannie
Copy link
Collaborator Author

Instead of removing n and event from observed, I think the better way to update the tests is to add these to expected.

Agreed! I'm uncertain about the origin of the "magic number" in the expected . I will reach out to John and inquire about the feasibility of including n (sample size) and event (number of events) in the expected object of test-unvalidated-sim_gs_n.R.

Hi @jdblischak, could you please let us know if it is possible to incorporate n and event in the expected object of the test-unvalidated-sim_gs_n.R?

@LittleBeannie LittleBeannie self-assigned this Feb 27, 2024
@LittleBeannie LittleBeannie added the development New feature or request label Feb 27, 2024
@jdblischak
Copy link
Collaborator

if it is possible to incorporate n and event in the expected object of the test-unvalidated-sim_gs_n.R?

@LittleBeannie Yes, I obtained expected by running constructive::construct(observed)

I'm uncertain about the origin of the "magic number" in the expected .

These are backwards compatibility tests. There is nothing magic about them. I cannot refactor the code without having tests to ensure I don't break anything.

@LittleBeannie
Copy link
Collaborator Author

if it is possible to incorporate n and event in the expected object of the test-unvalidated-sim_gs_n.R?

@LittleBeannie Yes, I obtained expected by running constructive::construct(observed)

I'm uncertain about the origin of the "magic number" in the expected .

These are backwards compatibility tests. There is nothing magic about them. I cannot refactor the code without having tests to ensure I don't break anything.

Thank you for your quick response! I encountered an issue while attempting to run constructive::construct(observed) directly in the console. It seems that I might have missed the correct way to run the code. Could you please help update the expected?

@jdblischak
Copy link
Collaborator

Could you please help update the expected?

Done in f815bbc

I encountered an issue while attempting to run constructive::construct(observed) directly in the console. It seems that I might have missed the correct way to run the code.

It's probably because we recently moved all the setup code to helper functions. To be able to use these, you have to first run devtools::load_all(".") in the console (Ctrl-Shift-L in RStudio). Annoyingly running devtools::test() in the console removes the helper functions, so you have to re-run devtools::load_all(".") to be able to return to interactive troubleshooting.

Copy link
Collaborator

@nanxstats nanxstats left a comment

Choose a reason for hiding this comment

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

Very nice, thanks for the update ✨

@nanxstats nanxstats merged commit 17ff585 into main Feb 27, 2024
7 checks passed
@nanxstats nanxstats deleted the 207-more-output-of-sim_gs_n branch February 27, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More output of sim_gs_n
3 participants