Adding possibility to select cim10 and atc in eds.cim10 and eds.drugs#314
Open
Adding possibility to select cim10 and atc in eds.cim10 and eds.drugs#314
Conversation
svittoz
force-pushed
the
terminology-subset
branch
from
55433ab to
c804921
Compare
Coverage Report
Files without new missing coverage
264 files skipped due to complete coverage. Coverage success: total of 97.76% is above 97.76% 🎉 |
|
percevalw
reviewed
Oct 7, 2024
|
|
||
| patterns = df.groupby("code")["patterns"].agg(list).to_dict() | ||
|
|
||
| patterns = {k: v for k, v in patterns.items() if k in cim10} if cim10 else patterns |
Member
There was a problem hiding this comment.
Why not (which would fail for unknown cim10 codes) and might be a bit faster
Suggested change
| patterns = {k: v for k, v in patterns.items() if k in cim10} if cim10 else patterns | |
| patterns = {k: patterns[k] for k in codes} if codes else patterns |
|
|
||
|
|
||
| def get_patterns() -> Dict[str, List[str]]: | ||
| def get_patterns(cim10: List[str] = None) -> Dict[str, List[str]]: |
Member
There was a problem hiding this comment.
Maybe standardize the cim10/atc into a "code" parameter ?
Suggested change
| def get_patterns(cim10: List[str] = None) -> Dict[str, List[str]]: | |
| def get_patterns(codes: List[str] = None) -> Dict[str, List[str]]: |
| name: str = "cim10", | ||
| *, | ||
| attr: str = "NORM", | ||
| cim10: List[str] = None, |
Member
There was a problem hiding this comment.
Suggested change
| cim10: List[str] = None, | |
| codes: List[str] = None, |
Comment on lines
+79
to
+81
| cim10 : str | ||
| List of cim10 to retrieve. If None, all cim10 will be searched, | ||
| resulting in higher computation time. |
Member
There was a problem hiding this comment.
Standardize cim10/atc/codes
Suggested change
| cim10 : str | |
| List of cim10 to retrieve. If None, all cim10 will be searched, | |
| resulting in higher computation time. | |
| codes : str | |
| CIM10 codes to retrieve. If None, synonyms for all codes will be searched | |
| resulting in higher computation time. |
| name=name, | ||
| regex=dict(), | ||
| terms=get_patterns(), | ||
| terms=get_patterns(cim10), |
Member
There was a problem hiding this comment.
Suggested change
| terms=get_patterns(cim10), | |
| terms=get_patterns(codes), |
| attr : str | ||
| The default attribute to use for matching. | ||
| atc : str | ||
| List of atc to retrieve. If None, all atc will be searched, |
Member
There was a problem hiding this comment.
Suggested change
| List of atc to retrieve. If None, all atc will be searched, | |
| codes : str | |
| ATC codes to retrieve. If None, synonyms for all codes will be searched | |
| resulting in higher computation time. |
| name=name, | ||
| regex=dict(), | ||
| terms=get_patterns(), | ||
| terms=get_patterns(atc), |
Member
There was a problem hiding this comment.
Suggested change
| terms=get_patterns(atc), | |
| terms=get_patterns(codes), |
|
|
||
|
|
||
| def get_patterns() -> Dict[str, List[str]]: | ||
| def get_patterns(atc: List[str] = None) -> Dict[str, List[str]]: |
Member
There was a problem hiding this comment.
Suggested change
| def get_patterns(atc: List[str] = None) -> Dict[str, List[str]]: | |
| def get_patterns(codes: List[str] = None) -> Dict[str, List[str]]: |
| with open(drugs_file, "r") as f: | ||
| return json.load(f) | ||
| patterns = json.load(f) | ||
| patterns = {k: v for k, v in patterns.items() if k in atc} if atc else patterns |
Member
There was a problem hiding this comment.
Suggested change
| patterns = {k: v for k, v in patterns.items() if k in atc} if atc else patterns | |
| patterns = {k: patterns[k] for k in codes} if codes else patterns |
| doc = matcher(doc) | ||
|
|
||
| assert len(doc.spans["measurements"]) == 0 | ||
| assert len(doc.spans["quantities"]) == 0 |
Member
There was a problem hiding this comment.
Add a backward compatibility test ?
Suggested change
| assert len(doc.spans["quantities"]) == 0 | |
| assert len(doc.spans["quantities"]) == 0 | |
| def test_measurements(blank_nlp): | |
| blank_nlp.add_pipe("eds.measurements") | |
| assert blank_nlp("La tumeur fait 4 cm").spans["measurements"][0]._.value.mm == 40 |
percevalw
force-pushed
the
master
branch
2 times, most recently
from
ae75dc5 to
430ef22
Compare
percevalw
force-pushed
the
master
branch
4 times, most recently
from
2038fb9 to
232ca91
Compare
percevalw
force-pushed
the
master
branch
3 times, most recently
from
fe81659 to
1ffa7c6
Compare
percevalw
force-pushed
the
master
branch
2 times, most recently
from
d2e1f39 to
65669dc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Currently,
eds.cim10andeds.drugsdoes not allow to select cim10 or ATC of interest, resulting in high computational time when used.TODO : see also
eds.umls?Checklist