Riksrevisjonen / pioneeR

R package for running a Shiny app for DEA analysis
https://riksrevisjonen.github.io/pioneeR/
GNU General Public License v3.0
6 stars 2 forks source link

35 update report #69

Closed ohjakobsen closed 9 months ago

ohjakobsen commented 9 months ago

This PR will (eventually) replace the old Rmarkdown report. However, in order to make the generation of the report more efficient an less error prone, I have chosen to move a lot of the logic from the server.R file to exported functions. As an added bonus, it will become much easier for the user to reproduce results. While the new report is not yet committed to the PR, it's a good idea to start the review process already.

Key questions:

Aeilert commented 9 months ago

This PR will (eventually) replace the old Rmarkdown report. However, in order to make the generation of the report more efficient an less error prone, I have chosen to move a lot of the logic from the server.R file to exported functions. As an added bonus, it will become much easier for the user to reproduce results. While the new report is not yet committed to the PR, it's a good idea to start the review process already.

Key questions:

  • Are the exported functions sensible in naming, arguments and output (keep in mind that we usually want data.frames)?
  • What would be sensible tests for these functions (some tests have already been added)?
  • Should more checks on inputs be added?

Great work @ohjakobsen! I have approved the PR. Added a minor type comment, but otherwise I think this is great. Lets keep working on tests and input checks in coming PRs. But this definitely moves us forward.