caravagnalab / CNAqc

CNAqc - Copy Number Alteration (CNA) Quality Check package
GNU General Public License v3.0
17 stars 8 forks source link

plot_multisample_CNA: code error in missing setting the blank_genome #12

Closed qingjian1991 closed 2 years ago

qingjian1991 commented 2 years ago

Hi:

When use the plot_multisample_CNA() for plotting multi-regional CNAs, I found some errors in your codes

The original code is: ` #line 62-66

Default blank genome -- remove labels with label_chr = NA

bl_genome = suppressMessages( CNAqc:::blank_genome(label_chr = NA) + labs(x = "", y = "") ) `

This code forgets to set some parameters. The corrected code may be:

bl_genome = suppressMessages(CNAqc:::blank_genome( ref = L[[1]]$reference_genome, chromosomes = chromosomes, label_chr = NA) + labs(x = "", y = ""))

In addition, I suggest adding some parameters in this function to control the output styles, like parameters of plot_segments as well as KARYO_colors and min_length_show ( the minimal length of chromosomes to show )

Yours sincerely,

Qingjian Chen

caravagn commented 2 years ago

Hi @qingjian1991

qingjian1991 commented 2 years ago

Hi @caravagn

Reply: Yes, the original code missed the reference.

Reply: I only set the two parameters. In addition, users maybe want to show the total copy numbers, rather than Karyotypes. I think this set would be helpful for users.

Hi @qingjian1991

  • are you suggesting we miss the reference? If yes I think you are correct.
  • upgrades are indeed possible, only those 2 would be required in your opinion?
caravagn commented 2 years ago

Makes sense. Do add the options and open another PR for me to check, happy to accept if everything works.

caravagn commented 2 years ago

Thanks @qingjian1991