Skip to content

Exploring enhancements to support val.meter transition#5

Draft
dgkf wants to merge 1 commit intomainfrom
4-exploratory-val-meter
Draft

Exploring enhancements to support val.meter transition#5
dgkf wants to merge 1 commit intomainfrom
4-exploratory-val-meter

Conversation

@dgkf
Copy link

@dgkf dgkf commented Jan 22, 2026

This PR is primarily illustrative. I did not do this work with the intention of opening a PR, but rather to test some enhancements. Let's use this PR to discuss changes, then I'll open separate PRs with more targeted features.

I also worked on this over the holidays when I had a bit of time to get nerd-sniped by knitr pain points, so there's are a few fancy tricks in here that aren't necessarily critical. I would not be opposed to scrapping them if they seem needlessly complex.

A few goals I had when making edits:

  • Make it easier to programmatically update the yaml frontmatter (will highlight the knitr trick I used in annotations)
  • Allow a more diverse set of inputs for the package metrics. All of these are handled simply by passing these various inputs through convert(<input>, val.meter::pkg). In order from simplest to most complex:
    • Package name (metrics computed for package during report rendering)
    • A list of package metrics (metrics pre-computed)
    • A dcf-formatted string (metrics pre-computed, possibly pulled from repo)
    • A package object Rds filepath (contains metrics, intermediate data and logs for the most comprehensive report)
  • The possibility of fetching logs captured when evaluating a metric
  • The ability to pass session info captured from another session. Useful, for example, if we want to record session info from a container used to evaluate a package, but build a report elsewhere.

@dgkf dgkf requested a review from llrs-roche January 22, 2026 20:14
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.

  • Make it easier to programmatically update the yaml frontmatter (will highlight the knitr trick I used in annotations)

Thanks

  • Allow a more diverse set of inputs for the package metrics. All of these are handled simply by passing these various inputs through convert(<input>, val.meter::pkg). In order from simplest to most complex:
  • Package name (metrics computed for package during report rendering)
  • A list of package metrics (metrics pre-computed)
  • A dcf-formatted string (metrics pre-computed, possibly pulled from repo)
  • A package object Rds filepath (contains metrics, intermediate data and logs for the most comprehensive report)

I think this is the most contentious changes here and the one I also have more problems with:
Quarto can render single files but assumes they are part of a bigger project directory and moves them.
Using parameters means that we cannot pass a list of parameters and in a single build step get all reports generated.
But having the quarto process calculate the metrics could interfere with the process, make them not programmatically accessible and slow the process.
I agree that it has the advantage of a tighter coupling between the report environment the metric environment and capture logs but that might not be necessarily positive.

I am not sure if we should support all type of input method or just restrict to some minimal support of input formats on the template provided by the package.

  • The possibility of fetching logs captured when evaluating a metric

See above.

  • The ability to pass session info captured from another session. Useful, for example, if we want to record session info from a container used to evaluate a package, but build a report elsewhere.

See above.

@dgkf
Copy link
Author

dgkf commented Jan 23, 2026

I think this is the most contentious changes here and the one I also have more problems with: Quarto can render single files but assumes they are part of a bigger project directory and moves them. Using parameters means that we cannot pass a list of parameters and in a single build step get all reports generated.

I'm not grasping the issue you're identifying here. As far as I can tell, we still render all the reports one-at-a-time and pass parameters individually. Notably passing assessments as an .Rds. We'd still be doing exactly that, but the .Rds would contain a pre-calculated val.meter::pkg object instead of a riskmetric::pkg_ref object.

But having the quarto process calculate the metrics could interfere with the process, make them not programmatically accessible and slow the process.

Just to be clear, the generalizations here are intended to support multiple workflows. Our current workflow (assessment -> results as .Rds -> report) would absolutely continue to be supported. Results are only calculated if the convert(params$package, val.meter::pkg) result has yet to calculate metrics.

@llrs-roche
Copy link
Collaborator

I'm not grasping the issue you're identifying here. As far as I can tell, we still render all the reports one-at-a-time and pass parameters individually.

Not necessarily an issue but some infelicity I would like to avoid. I think it would be faster if we create the data and have the template and build multiple times the template with each assessment of the package but in a single quarto process.
Why? because the build quarto builds a new _site each time and we need to move each report to the right place for the website and then build it again so that it can be searched/listed.

But I am not opposing to supporting more assessment input options, as long as the report (and GHA) don't need to deal with them.

@dgkf
Copy link
Author

dgkf commented Jan 26, 2026

@llrs-roche - Gotcha, yeah that use case is still certainly the priority with this change. The intention with the other input options is to ideally get the same (or as close to the same as possible) report whether the assessment data is pre-calculcated (Rds), calculated in that session (package name or filepath), or pulled from our repository (as DCF).

@llrs-roche
Copy link
Collaborator

Ok, let's then move to smaller PR(s) to make it easier to check them.

This was referenced Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants