New Feature: SNOMED::ICD10CM Mapping Support

[joeflack4]

Fixes #208

New Feature: SNOMED::ICD10CM Mapping Support

• Added feature to allow for conversion of these premade mappings provided by SNOMED into SSSOM format.

General updates

• utils.py: Reorganized SSSOM_READ_FORMATS: Top half are plain data formats, and bottom half are special-case formats. Both halves of the list are alphabetically sorted.

Docs

• Added a page parsers.rst, and populated with 'SNOMED complex map' info.

[matentzn]

The actual extension is looking great! @hrshdhgd can take it for a ride when the time comes.

Sorry we have not introduced you to our build system: we are using tox - please do not commit any requirements.txts here. To build sssom-py, just use tox. If you have questions on that @hrshdhgd will be happy to help!

[joeflack4]

Current output (updated 2022/03/03 4:25pm EST): sssom_map.tsv.zip

[hrshdhgd]

I did some formatting Joe, I hope that was fine. The pending errors from tox are as follows:

sssom/parsers.py:273:1: DAR003 Incorrect indentation: ~<
sssom/parsers.py:704:1: DAR003 Incorrect indentation: ~<
sssom/parsers.py:745:59: W605 invalid escape sequence '\.'
sssom/parsers.py:746:66: W605 invalid escape sequence '\?'

[hrshdhgd]

Fixed the errors:

sssom/parsers.py:745:59: W605 invalid escape sequence '\.'
sssom/parsers.py:746:66: W605 invalid escape sequence '\?'

by adding a r before the regex expression. Still the other 2 are pending. Source for solution.

I think the pending ones must have risen from using spaces instead of tabs in your IDE. Not sure, tried fixing it on my end, but it still persists.

[joeflack4]

@matentzn wrote:

I cannot review this with these spurious diffs.. Can we somehow avoid these?

Just to clarify, by spurious, do you mean the review comments that are linked to code snippets that are marked with the Outdated label?

I don't think that's really something that can easily be changed. The way to change that would be for me to write code once and for it to be perfect and then thus not have the need to change it. But I don't really think that's realistic. I think it's probably better to hack away at things fast, get something pushed, and then make future edits as needed; agile style.

[joeflack4]

@hrshdhgd Thanks so much for addressing the style / tox related changes. @matentzn @hrshdhgd honestly I totally forgot to run tox and make sure my code was compliant before re-pushing. Will eventually remember to do that every time, just need to make a habit.

[joeflack4]

@matentzn wrote:

I am super excited about this, and I am very happy about how much care you are taking sorting out the mapping here. Most of the things in here we need to discuss in person, too much to unpack to type up now, but two things would be probably good:

1. Start a new "parser" documentation as described in comments above
2. Rebrand the method to "snomed_map" rather than "snomed_icd10_map". In fact, there are two very different ones: complex and simple map. So lets figure out which of the two you are dealing with at the moment, see also: https://github.com/mapping-commons/sssom/issues/42 and rebrand your methods accordingly. So read_snomed_simple_map() or read_snomed_complex_map() depending on what we are dealing with here.

1. Yep, will do.

2.a. Simple vs Complex You must be looking at some old code. I remember we discussed this back in March I think, and I investigated and determined that I am using the complex map. I updated my code a few months ago; all of the methods and CLI related to this feature are named "complex". I have not utilized the simple map yet (not sure where to download it either), and that is not on my radar at the moment. I think we should probably merge this feature with only the complex map implementation, and we can always add simple later.

2.b. SNOMED map vs SNOMED::ICD map Ok, I can rename this. But just to clarify: Are you sure that SNOMED publishes other mappings that are in the same format as the SNOMED::ICD mappings? If not, then we should continue calling this "snomed_icd_map". But if they are in the same format, then yeah I agree renaming to just "snomed_map". I can look into this for you, but I don't want it to necessarily slow down this PR. The longer we wait to merge, the more time I'll have to spend rebasing my code.

[cthoyt]

Is there interest to finish this?

[joeflack4]

@cthoyt I hate to see unfinished work. I think I had basically finished "complex map" or "simple map". Other priorities took hold and this was forgotten. I leave it to @matentzn to decide, but perhaps the best thing to do is (i) close this PR, (ii) open an issue about finishing this, linking to this PR.

[matentzn]

@joeflack4 ok! Feel free to do As you see fit :)

[joeflack4]

@Nico OK then. The issue for this is still open:

• #208

I just updated it a little bit.

@cthoyt I could see this PR staying open for years / never getting done. So if you'd like to close it, feel free, and we can keep the above issue open.

[matentzn]

Let leave in draft stage, I can see this becoming important in mid 2024