-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ID Triplet Feature #146
base: main
Are you sure you want to change the base?
ID Triplet Feature #146
Conversation
9bee5d0
to
5ebabcd
Compare
a2c620c
to
e2a9f43
Compare
e2a9f43
to
a9bb607
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
==========================================
- Coverage 97.21% 96.92% -0.29%
==========================================
Files 30 31 +1
Lines 1327 1496 +169
==========================================
+ Hits 1290 1450 +160
- Misses 37 46 +9 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start @cbrinson-rise8
Co-authored-by: Eric Buckley <[email protected]>
Co-authored-by: Eric Buckley <[email protected]>
Co-authored-by: Eric Buckley <[email protected]>
Co-authored-by: Eric Buckley <[email protected]>
|
||
def __str__(self): | ||
""" | ||
Return the value of the enum as a string. | ||
""" | ||
return self.value | ||
return self.value | ||
class Feature(pydantic.BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chaning Feature from an enum to a pydantic.BaseModel means we can't auto list out all the choices in the API docs anymore for features. Since we already have an enum for the FeatureAttribtues and another for Identifier, did you consider just combining those to create a new Feature enum? For example
def all_features() -> typing.Iterator[str]:
"""
Return a list of all possible features that can be used for comparison.
"""
for feature in FeatureAttribute:
yield str(feature)
if feature == FeatureAttribute.IDENTIFIER:
for identifier in IdentifierType:
yield f"{feature}:{identifier}"
Feature = enum.Enum("Feature", [(f, f) for f in all_features()])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in an enum for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me share a slightly different implementation. I'm thinking we don't need both Feature and FeatureEnum, we can just use one class for both functionality and docs. I'm not married to this implementation as it requires us to "parse" everytime we call suffix() or attribute(), so it might be better to split the suffix and attribute in the constructor so we don't have to parse everytime the functions are invoked. However this should give you a better idea as how we can meet both needs with one class.
class Feature(enum.Enum):
"""
Enum for the different Patient attributes that can be used for comparison.
"""
# Dynamically populate the enum members
@classmethod
def populate(cls):
"""Populate the Feature enum dynamically."""
features = {}
for f in FeatureAttribute:
features[f.value] = f.value
if f == FeatureAttribute.IDENTIFIER:
for i in IdentifierType:
features[f"{f.value}:{i.value}"] = f"{f.value}:{i.value}"
# Add members to the class
cls._member_map_.update(features)
cls._value2member_map_.update({v: cls(k, v) for k, v in features.items()})
def attribute(self) -> FeatureAttribute:
"""Get the attribute of the feature."""
return FeatureAttribute(self.value.split(":")[0])
def suffix(self) -> typing.Optional[IdentifierType]:
"""Get the suffix of the feature if it is an identifier, otherwise None."""
if self.attribute() == FeatureAttribute.IDENTIFIER:
return IdentifierType(self.value.split(":")[1])
return None
def __str__(self):
"""Return the value of the enum as a string."""
return self.value
Feature.populate()
Co-authored-by: Eric Buckley <[email protected]>
Co-authored-by: Eric Buckley <[email protected]>
Co-authored-by: Eric Buckley <[email protected]>
@@ -83,9 +83,13 @@ linkage evaluation phase. The following features are supported: | |||
|
|||
: The patient's email address. | |||
|
|||
`DRIVERS_LICENSE` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MRN and SSN on lines 18 and 22 need to be removed as well
Description
Related Issues
closes #125
Additional Notes
/link
endpoint to accept an identifier tripletIDENTIFIER
valuesIDENTIFIER
and/orIDENTIFIER:XXX
values<--------------------- REMOVE THE LINES BELOW BEFORE MERGING --------------------->
Checklist
Please review and complete the following checklist before submitting your pull request:
Checklist for Reviewers
Please review and complete the following checklist during the review process: