Output VRS_Error INFO field for VCF sites where VRS computation fails #301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add try/except to top-level VCF record iterator Add error message generation to except block Re-raise errors in _get_vrs_object()
Separated out each test to make fixing errors easier Tightened passing criteria by asserting "all played" on the cassette
|
@korikuzma let me know if you have time to review and merge or if I should go ahead and do it. |
|
@larrybabb I'll take a look now. |
korikuzma
approved these changes
Dec 20, 2023
Comment on lines
140
to
149
| VCF_ESCAPE_MAP = { | ||
| "%": "%25", | ||
| ":": "%3A", | ||
| ";": "%3B", | ||
| "=": "%3D", | ||
| ",": "%2C", | ||
| "\r": "%0D", | ||
| "\n": "%0A", | ||
| "\t": "%09" | ||
| } |
There was a problem hiding this comment.
:, =, and \r are not used. Can we remove them? If so, then instead of
for search in [ "%", ",", ";", "\t", "\n" ]:
err_msg = err_msg.replace(search, self.VCF_ESCAPE_MAP[search])we can do
VCF_ESCAPE_MAP = {
"%": "%25",
";": "%3B",
",": "%2C",
"\n": "%0A",
"\t": "%09"
}
VCF_ESCAPE_MAP_KEYS = VCF_ESCAPE_MAP.keys()
...
for search in VCF_ESCAPE_MAP_KEYS:
err_msg = err_msg.replace(search, self.VCF_ESCAPE_MAP[search])There was a problem hiding this comment.
@ehclark do you agree with this feedback? If so, please submit the change if not, please explain why and I'll do the merge.
|
@larrybabb You're good to merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update the implementation that annotates a VCF with VRS IDs such that when an error occurs, a
VRS_ErrorINFO field is added to the site instead of the VRS INFO simply being left blank.The existing implementation caught and suppressed many exceptions at the translation step. These exceptions are still caught and logged, but then re-thrown to propagate to the top-level VCF site iterator.
With the new behavior, every VCF site will be modified according to the following:
VRS_ErrorINFO field will be added with the relevant error messageVRS_Allele_IDsINFO field will be added with the VRS ID for the alleles supported by the available translators and blank for those alleles not supportedPercent, newline, tab, semicolon and comma characters in the error messages are escaped with URL (percent) encoding according to the VCF spec.
The unit tests for VCF annotation have been split out into individual tests to make modifications to the VCR cassette fixtures easier to make and debug. Additional VCR configuration and assertions have been added to enforce a requirement that the cassettes must match exactly what is expected.
See also #297