EBIvariation / vcf-validator

Validation suite for Variant Call Format (VCF) files, implemented using C++11
Apache License 2.0
129 stars 39 forks source link

Added check to ignore symbolic alternate alleles #124

Closed ManeshNarayan closed 6 years ago

ManeshNarayan commented 6 years ago

Updated record_cache.hpp to check for and prevent the addition of symbolic alternate alleles to cache, as mentioned in issue #122.

jmmut commented 6 years ago

Also, did you try to compile the project locally? it's fine to do small changes (documentation, comments) directly in github, but that's not the general workflow we use. How familiar with git are you? do you know how to use branch, commit, pull, push and rebase?

ManeshNarayan commented 6 years ago

I have tried to integrate all the requested changes as suggested. I normally compile and run the program in the docker environment on my machine. I am familiar with git and the mentioned concepts. I will start actively using them.

ManeshNarayan commented 6 years ago

The run is failing but the error output on travis-ci shows lines from the upstream(older) vcf.ragel file instead of the vcf.ragel file which is part of the commit. What could be the issue here? It runs fine on my system

jmmut commented 6 years ago

I think the issue is that you didn't include in the commits the code that ragel generates.

As we explain in the readme the code generated by ragel (vcf-validator/inc/vcf/validator_detail_v41.hpp, v42 and v43) should be updated in the repository, so that it's not needed to install ragel to build the project.

However, once we need to modify the ragel code (vcf.ragel) we have to regenerate them with the lines:

ragel -G2 src/vcf/vcf_v41.ragel -o inc/vcf/validator_detail_v41.hpp
ragel -G2 src/vcf/vcf_v42.ragel -o inc/vcf/validator_detail_v42.hpp
ragel -G2 src/vcf/vcf_v43.ragel -o inc/vcf/validator_detail_v43.hpp
jmmut commented 6 years ago

well, now I remembered you used the docker compilation, and that should run the ragel commands for you. Then, it's just a matter of commiting the files validator_detail_v4*.hpp

ManeshNarayan commented 6 years ago

I added the validator_detail_v4*.hpp files. However the run is now failing with "No such file or directory" when test_validator is run. Building both the master branch from EBIvariation and the branch from my fork are giving the same error.

jmmut commented 6 years ago

I would say the issue you see is that you are running the command from the bin directory. in the readme there's a note explaining that the files that are used in the tests have a path from the root of the project. If you run the tests as ./bin/test_validator those should go.

however, the tests in travis are actually failing because of a segmentation fault. Let me have a look at those.

ManeshNarayan commented 6 years ago

Running it with ./bin/test_validator as mentioned in the README caused the failures. However I tried running it with ./build/bin/test_validator and it worked!

jmmut commented 6 years ago

hmm, that's weird, can you post what git status says? if travis failed and in my machine fails (with a segfault) it's likely that you didn't commit all changes

ManeshNarayan commented 6 years ago

Oh no, this was in reference to the previous comment for running the command for the master branch from EBIvariation. The test_validator for this branch runs correctly when executed with ./build/bin/test_validator.

The commit is throwing a segfault, as you pointed out. I should have been clearer. Apologies.

ManeshNarayan commented 6 years ago

The requested changes have been addressed. The cases in normalize_test.cpp don't contain structural variants, so the values for normalized_alternate_type haven't been entered for these, as they won't be utilized. That is fine right?

ManeshNarayan commented 6 years ago

Added test cases and made the suggested changes in this commit