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.

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

print power metrics #415

Merged
merged 1 commit into from
Jan 11, 2024
Merged

print power metrics #415

merged 1 commit into from
Jan 11, 2024

Conversation

RobFryer
Copy link
Contributor

Resolves #407

Allows users to print the extra power metrics for lognormally distributed data

  • the extra_output argument can be extended in the future to allow other metrics to be printed
  • resolved an inconsistency in the power calculations which I spotted as I went along
  • updated external data vignette
  • updated documentation
  • tested on OSPAR data

The code is not elegant, but does the job, and should suffice until the whole routine is over-hauled.

@morungos If you could review and approve today, then I will continue with the changes to the symbology in the summary table tomorrow.

Copy link
Contributor

@morungos morungos left a comment

Choose a reason for hiding this comment

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

One change flagged, I think a badge crept in a parameter comment. If you can remove that I'll happily merge.

@@ -392,9 +402,13 @@ ctsm.web.AC <- function(assessment_ob, classification) {
#' a csv file).
#' @param determinandGroups optional, a list specifying `labels` and `levels`
#' to label the determinands
#' @param classColour Specifies the colour scheme for the output symbology.
#' Will be changed soon.
#' @param classColour `r lifecycle::badge("experimental")` Specifies the
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a badge has crept in here, where it shouldn't be.

@morungos
Copy link
Contributor

Review done. If I am extra-grumpy today, blame the UK Passport Office 🤣. And I'm sorry.

@RobFryer
Copy link
Contributor Author

@morungos Passport offices can be a real pain!
The badge was intended, because the argument is likely to change with the updates to the symbology. Which I'm gong to do now!. I'm going to merge, and get on with the symbology changes. I might put the experimental badge on the function instead.

@RobFryer RobFryer merged commit a35b3ba into osparcomm:develop Jan 11, 2024
1 check passed
@RobFryer RobFryer deleted the print_power branch February 4, 2024 16:58
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.

Print power metrics
2 participants