-
Notifications
You must be signed in to change notification settings - Fork 2
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
Calculate PGA per chromosome; updates to prettyRainfallPlot #251
Conversation
…ild to annotate_svs within Rainfall plot
) | ||
} | ||
|
||
all_chr <- c(1:22,"X","Y") %>% matrix() %>% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine to leave like this but when migrating to the child repo I will change this to directly pull from the chr_coordinates
object defined just before this 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems fine to me!
#' | ||
#' calculate_pga_chr(this_seg = multi_sample_seg) | ||
#' | ||
calculate_pga_chr = function(this_seg, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can probably be an argument in the collate_pga? I think it is fine to keep as is but I can incorporate this into that function when migrating. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would work; I think I had a separate collate function elsewhere but inside pga would be most convenient.
stop( | ||
"Labeling SV is only supported when a particular chromosome or zoomed region is plotted." | ||
) | ||
sv_chromosome = 1:22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I trust this works for the chr-prefixed too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, should probably change to
sv_chromosome = standardize_chr_prefix(incoming_vector = c(1:22,"X","Y"), projection = projection)
scale_color_manual(values = get_gambl_colours("rainfall")) + | ||
ylab("log(IMD)") + | ||
ylab(expression(log[10](IMD))) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice trick with expression!
Thanks for the update! I will merge this and then will transfer the updates to the appropriate child, while also modifying some small things! |
The updates introduced in this PR are now migrated to the appropriate children repos: |
Pull Request Checklists
Important: When opening a pull request, keep only the applicable checklist and delete all other sections.
Checklist for all PRs
Required
I tested the new code for my use case (please provide a reproducible example of how you tested the new functionality)
I ensured all dplyr functions that commonly conflict with other packages are fully qualified.
This can be checked and addressed by running
check_functions.pl
and responding to the prompts. Test your code after you do this.devtools::document()
) and addedNAMESPACE
and all other modified files in the root directory and underman
.Optional but preferred with PRs
Checklist for New Functions
Required
I documented my function using Roxygen style.)
Adequate function documentation (see new-function documentation template for more info)
I have ran
devtools::document()
to add the newly created function to NAMESPACE (do not manually add anything to this file!).Example:
import
statement.Example:
Checklist for changes to existing code
I added/removed arguments to a function and updated documentation for all changed/new arguments
I tested the new code for compatibility with existing functionality in the Master branch (please provide a reprex of how you tested the original functionality)