-
-
Notifications
You must be signed in to change notification settings - Fork 2
📑 Reporter Refactor: Adds support for new teal_report and teal_card class
#308
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
Conversation
Code Coverage SummaryDiff against mainResults for commit: dd96ec0 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 1 suites 0s ⏱️ Results for commit dd96ec0. ♻️ This comment has been updated with latest results. |
teal_report and teal_card class
Signed-off-by: Marcin <[email protected]>
m7pr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 3 modules that uses srv_g_decorate and pws is handled there. Unsure if we also need to use set_chunks_dims for those modules. Plots looked good without it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the reporter functionality to support new teal_report and teal_card classes in the teal framework. The changes remove old "Add to reporter" implementations and replace them with a new unified reporting system.
- Adds utility functions for setting chunk attributes and dimensions on teal cards
- Refactors all module server functions to return teal_report objects with embedded teal_cards
- Removes legacy reporter button UI elements and server-side implementations
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| R/utils.R | Added utility functions for teal_card attribute management |
| man/ | Documentation for new utility functions |
| R/tm_*.R | Removed old reporter implementation and integrated new teal_card system |
| DESCRIPTION | Updated dependencies for new teal and teal.reporter versions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
averissimo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! 💯
But there's always the 1, I believe tm_g_heat_bygrade (where there's a comment) is not setting the dimensions
All others are added to report with code and figures!
Co-authored-by: Copilot <[email protected]> Signed-off-by: Marcin <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Marcin <[email protected]>
…ing/teal.osprey into teal_reportable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I believe there's a missing option that is no longer displayed in the reporter.
- Can we use
teal.code::eval_code(q, "library(dplyr)")without surroundingdplyrwith quotes? The code is more natural this way. - Please fix linter errors
edit: All figures and tables are added to report as they should! Once the issues here are fixed I think we are ready to merge
Co-authored-by: André Veríssimo <[email protected]> Signed-off-by: Marcin <[email protected]>
averissimo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Let's go! 💯
But before, please fix linter errors 🙏
Tested with
Fixes
Companion PRs:
teal_reportandteal_cardclass teal#1541{teal}module returns ateal_reportobject that extends fromteal_datateal.code#255{teal}module returns ateal_reportobject that extends fromteal_datateal.data#370teal_report,teal_cardclass and revamps reporter UI teal.reporter#331teal_reportandteal_cardclass teal.modules.general#884Changes description
teal_reportobject on every module