search

Bioconductor / VariantAnnotation

Annotation of Genetic Variants
https://bioconductor.org/packages/VariantAnnotation
23 stars 20 forks source link

update vcfFields() values for empty input #14

Closed Liubuntu closed 5 years ago

Liubuntu commented 5 years ago

Hi Val @vobencha ,

I have made some additional modifications on the vcfFields()$fixed output values, and now it works fine with an empty input. It may not be the most efficient way, so feel free to make any modifications on my change. I think we also need to make sure that the new commit get the version bump kept and synced to git.bioconductor.org, so that the nightly build will catch it. Thank you and let me know for any questions.

Best, Qian

## now                                                                                                                                                                                                             

vcfFields(VCFHeader())                                                                                                                                                                                             
CharacterList of length 4                                                                                                                                                                                          
[["fixed"]] character(0)                                                                                                                                                                                           
[["info"]] character(0)                                                                                                                                                                                            
[["geno"]] character(0)                                                                                                                                                                                            
[["samples"]] character(0)                                                                                                                                                                                         

## previously                                                                                                                                                                                                      

vcfFields(VCFHeader())                                                                                                                                                                                             
CharacterList of length 4                                                                                                                                                                                          
[["fixed"]] REF ALT QUAL FILTER                                                                                                                                                                                    
[["info"]] character(0)                                                                                                                                                                                            
[["geno"]] character(0)                                                                                                                                                                                            
[["samples"]] character(0)                                                                                                                                                                                         
>
vobencha commented 5 years ago

@Liubuntu please explain what you mean by "I think we also need to make sure that the new commit get the version bump kept and synced to git.bioconductor.org, so that the nightly build will catch it"

Liubuntu commented 5 years ago

Hi Val,

There were 3 total pull request I have made to VariantAnnotation.

1st (https://github.com/Bioconductor/VariantAnnotation/commit/b36cfa524ca13135c9f00bafb541be75ed391707) is to add the vcfFields generic and method. 2nd (https://github.com/Bioconductor/VariantAnnotation/commit/ade795350c91ec9b85b4b0bcb2c2ac87e9f30077) is to change the returned value from SimpleList() into ChararcterList(). 3rd (https://github.com/Bioconductor/VariantAnnotation/commit/64bca43bd3ef5f76c0e36c34691a151c368f79de) is to modify the vcfFields()$fixed returned value. 4th (https://github.com/Bioconductor/VariantAnnotation/pull/14) is to modify the vcfFields()$fixed returned value again to be compatible with empty input, e.g., vcfFields(VCFHeader())$fixed.

It was about the 3rd commit , which you have merged (https://github.com/Bioconductor/VariantAnnotation/commit/26713c74a90f5fde959f1de2783ac3bd28ceda09) yesterday for the R script, but skipped the version bump. the daily build report shows VariantAnnotation 1.27.7(http://bioconductor.org/checkResults/devel/bioc-LATEST/VariantAnnotation/) and Last Changed Date: 2018-10-13, meaning it was not able to catch that commit for modification of the vcfFields()$fixed returned value, right?

That was not a bad thing actually because the modifications were not complete, and now I'm modififying it again to be compatible with empty VCFHeader() as input. So if we agree with the new changes, the version bump should be included also so that the building machine could catch it and build it.

Please correct me if anything is not right. I am not expert about the night build timelines and the sync between github and git.bioconductor.org. Was confused yesterday when I submit the VCFArray package but it could not build successfully. And Lori explained a little bit to me. Thanks!

Best, Qian

vobencha commented 5 years ago

This is my understanding of the situation:

commit 64bca43bd3ef5f76c0e36c34691a151c368f79de Author: Qian Liu qliu7@buffalo.edu Date: Mon Oct 15 10:48:16 2018 -0400

updated the returned values for vcfFields()

DESCRIPTION R/methods-VCFHeader-class.R


- I then updated the NEWS file on Wed Oct 17 before the builds started. This bumped the version from 1.27.7 to 1.27.8:

~/git.bioconductor.org/software/VariantAnnotation >git diff 26713c74a90f5fde959f1de2783ac3bd28ceda09 DESCRIPTION diff --git a/DESCRIPTION b/DESCRIPTION index 94ee1a6..494145b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -3,7 +3,7 @@ Type: Package Title: Annotation of Genetic Variants Description: Annotate variants, compute amino acid coding changes, predict coding outcomes. -Version: 1.27.7 +Version: 1.27.8 Authors@R: c(person("Valerie", "Obenchain", role=c("aut", "cre"), email="maintainer@bioconductor.org"), person("Martin", "Morgan", role="aut"),


- Version 1.27.8 is broken on today's build report due to a failing unit test for vcfFields(). The version on the build report is 1.27.8 but this failure is not related to my change to the NEWS file - it's related to the pull request that bumped the version to 1.27.7.

Do you know why the unit test is failing? Before more pull requests are accepted we need to understand why the unit test broke.

Liubuntu commented 5 years ago

Hi Val,

Thanks for the explanation. So 2 commits yesterday before the build starts, all with version bumps. So my confusion about different versions on github / git.bioconductor.org was clear now and we do normally need a day for the commit to pass to build. And the build report generating date / snapshot date / last changed date now make better sense to me.

That error indeed came from the version bump I made to 1.27.7, and it was fixed from today's pull request. (https://github.com/Bioconductor/VariantAnnotation/pull/14/files#diff-edd14eeb22fc7a7709c18b51a181ad94). I have tested and it could pass checks now. Sorry for messing up the commit messages...

Thanks! Qian

vobencha commented 5 years ago

@Liubuntu The package is clear in today's report. Thanks for the fix!

Liubuntu commented 5 years ago

Thank you Val. So the WARNINGS: are ok, right? I have seen the similar warnings in my package ranges also..