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

Fixing internal call of get_sample_cn_segments in examples #17

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

mattssca
Copy link
Contributor

@mattssca mattssca commented Nov 17, 2023

This tiny PR fixes an issue with the examples in subset_cnstates that calls get_sample_cn_segments with old parameters. This has been resolved in this PR. The issue was revealed during beta testing of the new GAMBLR structure.

@vladimirsouza vladimirsouza changed the title Fixing internal call of get_sample_cn_states in examples Fixing internal call of get_sample_cn_segments in examples Nov 18, 2023
@vladimirsouza
Copy link
Contributor

Could you fix the commit title by changing get_sample_cn_states to get_sample_cn_segments?

@vladimirsouza
Copy link
Contributor

This example create empty objects

> cn_states = get_sample_cn_segments(these_sample_ids = c("00-15201_tumorA",
+                                                         "HTMCP-01-06-00422-01A-01D"),
+                                    streamlined = FALSE)
Using the bundled CN segments (.seg) calls in GAMBLR.data...
Using the bundled metadata in GAMBLR.data...
> cn_states
# A tibble: 0 × 7
# ℹ 7 variables: ID <chr>, chrom <chr>, start <dbl>, end <dbl>, LOH_flag <dbl>, log.ratio <dbl>,
#   CN <dbl>

@mattssca
Copy link
Contributor Author

mattssca commented Nov 20, 2023

The issue with the example has been corrected to use sample IDs that are available in the bundled data for compatibility with GAMBLR.data. Note, the documentation is not updated since the affected function has the @noRd tag.

> head(get_sample_cn_segments(these_sample_ids = c("02-13135T",
+                                                         "SU-DHL-4"),
+                                    streamlined = FALSE), 10)
Using the bundled CN segments (.seg) calls in GAMBLR.data...
Using the bundled metadata in GAMBLR.data...
# A tibble: 10 × 7
   ID        chrom     start       end LOH_flag log.ratio    CN
   <chr>     <chr>     <dbl>     <dbl>    <dbl>     <dbl> <dbl>
 1 02-13135T 1         10001    762600        0     0         2
 2 02-13135T 1        762601 121500000        0     0         2
 3 02-13135T 1     142600000 161506889        0     0         2
 4 02-13135T 1     161506890 161652716        0     0         2
 5 02-13135T 1     161652717 162110568        0     0.728     3
 6 02-13135T 1     162110569 162111399        0     0         2
 7 02-13135T 1     162111400 249203757        0     0         2
 8 02-13135T 1     249203758 249250620        0     0         2
 9 02-13135T 2         10001     11319        0     0         2
10 02-13135T 2         11320  90500000        0     0         2

@vladimirsouza
Copy link
Contributor

I think this subset_cnstates function could be tweaked in the future. When run, it creates some objects in the global environment.

> rm(list=ls())
> devtools::load_all("/home/vsouza/repos/gamblr_repos/test_branches/GAMBLR.helpers/pr17/GAMBLR.helpers")
> cn_states = get_sample_cn_segments(these_sample_ids = c("02-13135T",
+                                                         "SU-DHL-4"),
+                                    streamlined = FALSE)
> ls()
[1] "cn_states"
> subset_cnstates(cn_segments = cn_states,
+                 samplen = 1)
> ls()
[1] "cn_1_sample1" "cn_3_sample1" "cn_4_sample1" "cn_5_sample1" "cn_6_sample1" "cn_states"   

As a consequence, when we run fancy_multisamp_ideogram, which internally runs subset_cnstates, these objects are also created in the global env, which probably is not a desirable but weird behaviour.

> rm(list=ls())
> library(GAMBLR.viz)
> fancy_multisamp_ideogram(these_sample_ids = c("05-18426T", "05-18426T"),
+                          include_cn2 = TRUE,
+                          plot_title = "Multi-sample Ideograms Example",
+                          plot_sub = "grch37",
+                          chr_anno_dist = 2.5,
+                          chr_select = paste0("chr", c(1:22)))
> ls()
[1] "cn_0_sample1" "cn_1_sample1" "cn_2_sample1" "cn_3_sample1" "cn_4_sample1"

If this is important, I could change it to not leave these "leftover" objects, or maybe just create an issue. What do you think?

@mattssca
Copy link
Contributor Author

Thanks for reviewing Vlad. I think the suggested update would be nice and it can be addressed in an upcoming PR. I will create an issue of this for tracking purposes.

@mattssca mattssca merged commit 1412c54 into main Nov 22, 2023
@mattssca mattssca deleted the cmattsson_hotfix branch November 22, 2023 23:53
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