Skip to content
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

Implement Validation for String-Float Conversions in Quantity Class #68

Open
jenniferjiangkells opened this issue Sep 30, 2024 · 5 comments
Labels
Component: CDA/FHIR parser Issue/PR that relate to the CDA or FHIR parsing component good first issue Good for newcomers hacktoberfest Issues suitable for hacktoberfest

Comments

@jenniferjiangkells
Copy link
Member

Description

The Quantity class in the healthchain/models/data/concept.py file currently allows both string and float values for the value field. We need to implement validation for conversions between string and float representations to ensure data consistency and prevent potential errors.

Context

This validation is important for maintaining data integrity within the Quantity class, which is used in various concepts such as medication dosages and ranges. Proper validation will help prevent issues related to data type mismatches and ensure that all quantity values are consistently represented and can be reliably used in calculations or comparisons.

Possible Implementation

  1. Add a custom validator to the Quantity class using Pydantic's @validator decorator.
  2. Implement logic to convert string inputs to float when possible.
  3. Raise a ValueError with a descriptive message if the conversion fails.
  4. Ensure that float values are properly formatted when converted to strings.
# healthchain/models/data/concept.py
class Quantity(DataType):
    value: Optional[Union[str, float]] = None
    unit: Optional[str] = None

    @validator('value')
    def validate_value(cls, v):
        if isinstance(v, str):
            try:
                return float(v)
            except ValueError:
                raise ValueError(f"Invalid value for Quantity: '{v}'. Must be a valid number.")
        return v

    def str(self):
        if self.value is not None:
            return f"{self.value:.2f} {self.unit or ''}"
        return ""

Possible Alternatives

  1. Restrict the value field to accept only float inputs, removing the string option entirely.
  2. Implement a custom data type that handles both string and float representations internally while exposing a consistent interface.
  3. Use a third-party library specialized in handling quantities and units, such as pint, which could provide more robust unit conversion and representation capabilities.
@jenniferjiangkells jenniferjiangkells added good first issue Good for newcomers hacktoberfest Issues suitable for hacktoberfest Component: CDA/FHIR parser Issue/PR that relate to the CDA or FHIR parsing component labels Sep 30, 2024
@guco44
Copy link

guco44 commented Oct 1, 2024

Hello, can you assign me this issue?

@guco44
Copy link

guco44 commented Oct 1, 2024

Thank you for assigning me the issue. I will start working on it next week. I tried to join your Discord community for communication, but the link appears to be invalid.

@jenniferjiangkells
Copy link
Member Author

👋 Hey thanks for contributing! Will update the Discord link - this one should work!

@Vortex-21 Vortex-21 mentioned this issue Oct 4, 2024
5 tasks
@adamkells
Copy link
Contributor

Thank you for assigning me the issue. I will start working on it next week. I tried to join your Discord community for communication, but the link appears to be invalid.

Hi @guco44 , thank you so much for taking an interest in contributing to healthchain, we really appreciate it! It looks like someone else may have already tackled this one unfortunately, we'd be happy if there's any other open issues you'd like to work on. We're hoping to add some more over the weekend too, so there should be lots more to work on next week.

@Vortex-21
Copy link
Contributor

@adamkells I have added the test file and two more checks in the Quantity Class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: CDA/FHIR parser Issue/PR that relate to the CDA or FHIR parsing component good first issue Good for newcomers hacktoberfest Issues suitable for hacktoberfest
Projects
Status: Todo
Development

No branches or pull requests

4 participants