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

Closes #1937 Display only results of metrics where data is available in reports #1953

Merged
merged 1 commit into from
Nov 20, 2024
Merged

Closes #1937 Display only results of metrics where data is available in reports #1953

merged 1 commit into from
Nov 20, 2024

Conversation

zdz2101
Copy link
Collaborator

@zdz2101 zdz2101 commented Nov 19, 2024 •
edited
Loading

Overview

Try "removing" kri0001 and cou0001 from the analysis data in example 3.1

# Step 3 - Create Reporting Layer - create reports using metrics data
reporting_wf <- MakeWorkflowList(strPath = "workflow/3_reporting")
reporting <- RunWorkflows(reporting_wf, c(mapped, list(lAnalyzed = analyzed[-c(1,13)],
                                                       lWorkflows = metrics_wf)))

@zdz2101 zdz2101 linked an issue Nov 19, 2024 that may be closed by this pull request
Bugfix: When metrics don't have any data, don't print the header in the report #1937
Closed
zdz2101
zdz2101 commented Nov 19, 2024
View reviewed changes
inst/report/Report_KRI.Rmd
@@ -126,7 +126,7 @@ Report_OverviewText(

```{r, echo=FALSE, results='asis'}

for (i in unique(params$dfMetrics$MetricID)) {
for (i in unique(params$dfResults$MetricID)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this really is the simplest way to fix it @lauramaxwell @samussiah , other "checks" may include making sure the metrics dataset coming out of 3_reporting/metrics.yaml only has metric IDs of the results?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh nice- i think this makes sense! way easier than what i was thinking we'd have to do 🎉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unless we want to put in additional guardrails, I think this is actually it

Copy link
Contributor

@samussiah samussiah Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

👌 Makes sense to me!

@zdz2101 zdz2101 marked this pull request as ready for review November 19, 2024 22:02
lauramaxwell
lauramaxwell approved these changes Nov 20, 2024
View reviewed changes
Copy link
Contributor

@lauramaxwell lauramaxwell left a comment

Choose a reason for hiding this comment

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

looking good!

@lauramaxwell lauramaxwell merged commit 5e90344 into dev Nov 20, 2024
6 checks passed
@lauramaxwell lauramaxwell deleted the fix-1937 branch November 20, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samussiah samussiah samussiah left review comments

@lauramaxwell lauramaxwell lauramaxwell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bugfix: When metrics don't have any data, don't print the header in the report
3 participants
@zdz2101 @samussiah @lauramaxwell