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

Exceptions for v2 to v3 conversion #291

Open
lcreteig opened this issue Dec 5, 2023 · 6 comments
Open

Exceptions for v2 to v3 conversion #291

lcreteig opened this issue Dec 5, 2023 · 6 comments

Comments

@lcreteig
Copy link

lcreteig commented Dec 5, 2023

I noticed that the py-ard implementation for converting v2 to v3 typings is not yet complete:

py-ard/pyard/db.py

Lines 586 to 600 in d43a035

# TODO: Create mapping table using both the allele list history and
# deleted alleles as reference.
# Temporary Example
v2_to_v3_example = {
"A*0104": "A*01:04:01:01N",
"A*0105N": "A*01:04:01:01N",
"A*0111": "A*01:11N",
"A*01123": "A*01:123N",
"A*0115": "A*01:15N",
"A*0116": "A*01:16N",
"A*01160": "A*01:160N",
"A*01162": "A*01:162N",
"A*01178": "A*01:178N",
"A*01179": "A*01:179N",
"DRB5*02ZB": "DRB5*02:UTV",

I was wondering if this is still on the roadmap and/or if I can contribute anything to it.

The heuristic conversion in ard._predict_v3() does not work in all cases, because there's a bunch of exceptions. For example, if I apply it to the Current Name column of the IPD-IMGT/HLA pre-2010 nomenclature file, I get a different result than in the Name as of April 2010 column in 1,045 out of 4,826 cases.

I've made a mapping table such as in the linked snippet above for my own use case, based on the following files:

Can you think of any other exceptions that should be included? In that case I'd really appreciate your feedback.

I'd be happy to share the mapping table, or the code that creates it (I have this in R now, but could probably translate it to Python) if that's of any use to you.

@pbashyal-nmdp
Copy link
Contributor

Hi Leon,

There wasn't a clear V2 to V3 standard mapping file available so it wasn't fleshed out properly. We'll review your approach. If you are able to, you can share you R script (no need to convert it). Thanks for following up on this.

Organization have their own mapping so we do support having your own V2 to V3 mappings using pyard-import's --v2-to-v3-mapping option. You can provide a CSV with 2 columns(V2 version, V3 version without column headers). See pyard-import section in README.

$ pyard-import  --v2-to-v3-mapping map2to3.csv

@pbashyal-nmdp
Copy link
Contributor

Looks like the current version is failing for --v2-to-v3-mapping option with the error:

AttributeError: 'tuple' object has no attribute 'cursor'

I'll have an update soon that should fix the issue.

@pbashyal-nmdp
Copy link
Contributor

Just released 1.0.8. If you upgrade or pip install py-ard==1.0.8, --v2-to-v3-mapping option should work with a mapping file.

@lcreteig
Copy link
Author

lcreteig commented Dec 6, 2023

A I didn't know about about --v2-to-v3-mapping flag, that's helpful, then I could still use my mapping table together with the heuristic prediction. Here's the table in case you're interested: map2to3.csv. Also, here's the R code that generated (something like) it.

I tried it with py-ard 1.0.8., database version 3540, and the above mapping table. This works for almost all cases, but strangely enough there's a handful (61) of cases in the IPD-IMGT/HLA pre-2010 nomenclature file where pyard seems to simply return the v2 typing, even though it's v3 equivalent is in the mapping table. This might be something on my end, but in case you'd like to try and reproduce it:

For example:

pyard-import --imgt-version 3.54.0  --v2-to-v3-mapping map2to3.csv
import pyard
ard = pyard.init('3540')
ard.v2_to_v3("A*020113") # works fine
# >>> 'A*02:01:13'
ard.v2_to_v3("A*020114") # just returns v2
# >>> 'A*020114'

@pbashyal-nmdp
Copy link
Contributor

Awesome. Thanks, I'll take a look.

@pbashyal-nmdp
Copy link
Contributor

Looks like A*02:01:14 is not a valid allele for 3540 db version.
I think it should skip this test in non-strict mode. I'll update it to use the mapping when in non-strict mode.

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

No branches or pull requests

2 participants