Skip to content

Fix html format and content#35

Merged
llrs-roche merged 6 commits intomainfrom
oriol.fix-html-report
Jun 9, 2025
Merged

Fix html format and content#35
llrs-roche merged 6 commits intomainfrom
oriol.fix-html-report

Conversation

@osenan
Copy link
Contributor

@osenan osenan commented Jun 4, 2025

Two errors were detected in the html report:

  1. There was one variable named x within a anonymous function but referring to object summary_data. That was making the table not to render in html format.
  2. Transformations in content of the summary_table that were expecting numeric format. Added check to verify is numeric, if not we are not transforming.

In addition, added a function to display "No" in the table in certain verifications that otherwise are "Yes" in green.

Finally added a test that should fail if we introduce changes to the report that return an error.

@osenan osenan requested a review from llrs-roche June 4, 2025 01:56
@osenan
Copy link
Contributor Author

osenan commented Jun 4, 2025

Here it is the report for the package it was failing
validation_report_accelmissing_v2.2.md

To generate in html run this code:

library(riskmetric)
# assuming you download the package source
ref <- pkg_ref("~/Downloads/accelmissing_2.2/accelmissing", source = "pkg_source")
assessment <- pkg_assess(ref)
saveRDS(assessment, file = "path_to_assessment/accell_assessment.rds")

Then create the report:

 library(riskreports)

  options("riskreports_output_dir" = "your_desired_path")

  pr <- package_report(
    package_name = "accelmissing",
    package_version = "2.2",
    params = list(
      assessment_path = "path_to_assessment/accell_assessment.rds",
      source = "pkg_install"),
    quiet = FALSE, # To silence quarto output for readability,
    output_format = "all"
    )

Copy link
Collaborator

@llrs-roche llrs-roche left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the report, I have messed it up while merging the multiple branches. One minor comment to address (or not if you feel strongly against it)

But now the summary table doesn't show up locally on the html format:

image

@dgkf dgkf moved this to In Progress in Package Development Jun 5, 2025
@rabiibouhestine rabiibouhestine moved this from In Progress to In Review in Package Development Jun 9, 2025
@llrs-roche
Copy link
Collaborator

Apologies @osenan and @rabiibouhestine I installed again the package and tried to generate only the html format and the table appeared. Not sure if the library path was messing with the version of riskreports being used or what...

@llrs-roche llrs-roche merged commit 6dc2e7a into main Jun 9, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Package Development Jun 9, 2025
Zhenglei-BCS pushed a commit that referenced this pull request Oct 9, 2025
Co-authored-by: osenan <oriol@praenoscere.com>
Co-authored-by: Rabii Bouhestine <rabiibouhestine@outlook.com>
Zhenglei-BCS pushed a commit that referenced this pull request Oct 9, 2025
Co-authored-by: osenan <oriol@praenoscere.com>
Co-authored-by: Rabii Bouhestine <rabiibouhestine@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants