-
Notifications
You must be signed in to change notification settings - Fork 6
fix(id_mapping): fix typo taxId -> taxid #40
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
Conversation
|
Okay that test failure isn't your fault. Can I have a test for this though please? |
|
I believe there is already a test for that: Unipressed/test/test_id_mapping/test_id_mapping.py Lines 163 to 174 in 4734fef
|
|
Ah I see. I'll have to test that this is broken on the master branch first then. |
|
Hmm, the tests are working on master (minus the flaky ID mapping one), so I think the current approach is correct In the API docs here it is shown as |
|
This is the code I am using: It was not working for me at first. However, after this change, it started working. I took inspiration from here, where it is described as "taxid" not "taxId". |
That's using a different (I presume older) API than Unipressed uses
I wonder if that's because your taxon ID doesn't match your uniprot IDs, and therefore the change you proposed removes the taxon filter and gets you your expected results. I would double check your inputs. I'm pretty sure the current approach in Unipressed is correct. |
|
@multimeric, this is the error I get directly from the request (without my change): I don't think it is related to the taxon IDs not matching. Try running this code. Let me know if it works for you (P38398 comes from human BRCA1). request = IdMappingClient.submit(
taxon_id=9606, source="UniProtKB_AC-ID", dest="Ensembl_Protein", ids=['P38398']
)(BTW, I am using Python 3.12.7) |
|
Hmm true enough, something is going on here. My best guess is that it relates to how you're doing Can you please add your example above as a new test case. |
|
I have sent this issue to UniProt support. Ticket |
|
Hi @multimeric , |
|
Thanks @ahmadshadab. Looks like I'll have to tweak the interface and docs to make this clear to users. I think the current behaviour is okay in the meantime. |
As described in title. This is the error I get directly from the request instance (without my change):
Steps to reproduce the problem (P38398 comes from human BRCA1):