medact(feat): cdb utils for merging and navigation using pt2ch relations #176
Conversation
fix mypy errors last mypy fix fix lint error
|
Looks like now it's just a failure of the new test. Seems like it was failing before as well, but it got buried under everything else that was happening workflow wise. |
|
ah yep will fix. Looks like an issue with the test CDB. |
| ) | ||
|
|
||
| # Should have 3 concepts total | ||
| self.assertEqual(len(merged_cdb.cui2info), 3) |
There was a problem hiding this comment.
Could do this check dynamic - add together the sets of keys from both and len them for comparison.
medcat-v2/medcat/utils/cdb_utils.py
Outdated
| all_links = [] | ||
| if cui not in cuis2nodes: | ||
| # Get preferred name from the new CDB structure | ||
| preferred_name = cdb.cui2info.get(cui, {}).get('preferred_name', cui) |
There was a problem hiding this comment.
Could just use cdb.get_name for this.
medcat-v2/medcat/utils/cdb_utils.py
Outdated
| 'links': all_links | ||
| } | ||
| except KeyError as e: | ||
| logger.warning(f'Cannot find path concept path:{e}') |
There was a problem hiding this comment.
Would be better to include exception in exc_info=e instead of printing out only the message.
Or if only the message is important, then format in a lazy way (i.e include %s in string and pass e as argument like "...path: %s", e)).
| # Should have 3 concepts total | ||
| self.assertEqual(len(merged_cdb.cui2info), 3) | ||
|
|
||
| # CUI1 should be merged |
There was a problem hiding this comment.
This looks like it's all hard coded. Couldn't we just iterate over a) both the merged CDBs and b) all CUIs in each, and then do the checks under a self.subTest?
Wouldn't need a separate section for each CUI that way.
| ) | ||
|
|
||
| # Should be identical to cdb2 | ||
| self.assertEqual(len(merged_cdb.cui2info), len(self.cdb2.cui2info)) |
There was a problem hiding this comment.
Shouldn't the entire dicts be equal If there's nothing added from the empty one?
2 participants
medcat_utils.py, documented and tested , as they are to be used elsewhere