[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:

bioservices/src/bioservices/uniprot.py

Line 476 in 07577b6

results = self.services.http_get(f"idmapping/status/{job_id}", frmt="json")

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:

bioservices/src/bioservices/uniprot.py

Line 482 in 07577b6

fails = results["failedIds"]

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:

bioservices/test/webservices/test_uniprot.py

Lines 46 to 52 in cce05ab

	def test_mapping(uniprot):
	assert "KEGG" in uniprot.valid_mapping
	res = uniprot.mapping("UniProtKB_AC-ID", "KEGG", "P43403,P123456")
	res = uniprot.mapping("UniProtKB_AC-ID", "KEGG", ["P43403", "P123456"])
	assert len(res["results"]) == 1
	assert len(res["failedIds"]) == 1

[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'] == []