[Bug in v1.11.0] Need to check for the presence of `failedIds` key in uniprot.mapping

[taoualiw]

Bug introduced in v1.11.0

It seems that the result of http_get(f"idmapping/status/{job_id}", frmt="json") does not contain the failedIds key if there are no failed ids: https://github.com/cokelaer/bioservices/blob/07577b67a58e45fccc9bb8a94d6aa41b62787031/src/bioservices/uniprot.py#L476

The changes introduced in the most recent release v1.11.0, 16 hours ago result in a KeyError bug when that happens and the line responsible for that is the following: https://github.com/cokelaer/bioservices/blob/07577b67a58e45fccc9bb8a94d6aa41b62787031/src/bioservices/uniprot.py#L482

See the following example

In [1]: from bioservices.uniprot import UniProt

In [2]:     u = UniProt(verbose=False, cache=False)
   ...:     u.services.logging.setLevel("ERROR")

In [3]: u.mapping("UniProtKB_AC-ID", "KEGG", "P43403,P1
   ...: 23456")
0it [00:00, ?it/s]
Out[3]: {'results': [{'from': 'P43403', 'to': 'hsa:7535'}], 'failedIds': ['P123456']}

In [4]: u.mapping("UniProtKB_AC-ID", "KEGG", "P43403")
-------------------------------------------------------
KeyError              Traceback (most recent call last)
<ipython-input-4-9022bf9c0b6c> in <module>
----> 1 u.mapping("UniProtKB_AC-ID", "KEGG", "P43403")

~/opt/miniconda3/envs/my_env/lib/python3.7/site-packages/bioservices/uniprot.py in mapping(self, fr, to, query, polling_interval_seconds, max_waiting_time, progress)
    480                 batches = results["results"]
    481 
--> 482                 fails = results["failedIds"]
    483 
    484                 size = 25

KeyError: 'failedIds'

Unfortunately, the existing test does not exercise the case where there are no failed ids: https://github.com/cokelaer/bioservices/blob/cce05ab8e6b0530c3e3200ff143844b7a6865cfc/test/webservices/test_uniprot.py#L46-L52

[cokelaer]

@taoualiw thanks. I have fixed the issue locally with a new tests and will push a release during the day. thanks again for reporting this issue. I'll close the issue once the new release is available on pypi

[taoualiw]

Thanks @cokelaer for the quick fix and release. Everything works fine now. However, I would like to bring to your attention that the newly added regression test wasn't exercising the issue. The failing example was u.mapping("UniProtKB_AC-ID", "KEGG", "P43403")( not u.mapping("UniProtKB_AC-ID", "KEGG", "P43403,P123456")).

[cokelaer]

@taoualiw thanks for the feedback. I improved the test

res = uniprot.mapping("UniProtKB_AC-ID", "KEGG",' P43403,P123456")
assert res['failedIds']
# here no failedId but we expect an empty failedIds in the returned dictionary (empty list)
res = uniprot.mapping("UniProtKB_AC-ID", "KEGG", "P43403")
assert res['failedIds'] == []