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

move table sort/prune into post processing #314

Closed
clarkliming opened this issue Nov 8, 2022 · 2 comments · Fixed by #316
Closed

move table sort/prune into post processing #314

clarkliming opened this issue Nov 8, 2022 · 2 comments · Fixed by #316
Assignees
@clarkliming

Comments

@clarkliming
Copy link
Contributor

part of #306
@clarkliming clarkliming self-assigned this Nov 8, 2022
@BFalquet
Copy link
Contributor

BFalquet commented Nov 8, 2022 •
edited
Loading

Note of the discussion with @clarkliming :

Let's consider the idea to make the output of the main function a list (of rtables, graph or listings) to facilitate pruning in the postproces step and increase consistency of output type.

@clarkliming
Copy link
Contributor Author

clarkliming commented Nov 9, 2022 •
edited
Loading

I am thinking of decorators (in Python); but this can be difficult for users.

postprocessing = list(smart_prune, null_report)

then we call smart_prune first, then check if it is null report

but now I will only move the prune/sort part to post_processing

@clarkliming clarkliming mentioned this issue Nov 9, 2022
Merged
BFalquet pushed a commit that referenced this issue Nov 10, 2022
@clarkliming
complete refactor of postprocess (#316)
c523408 
close #314 

move prune/sort/null_report to post processings

other things changed:
1. move scda to imports (avoid errors when user try to run examples
without scda)
2. create data with syn_test_data to increase the speed
3. add helper script for testthat to avoid repeated call of
syn_test_data()
4. create null_report object and export it
5. put "no observations.." in content instead of header


decorations, should also go into "post-processings" but currently we
have them in main. we can always change it later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@clarkliming clarkliming

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

complete refactor of postprocess insightsengineering/chevron
2 participants
@clarkliming @BFalquet