-
Notifications
You must be signed in to change notification settings - Fork 16
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
Overflow and Type Errors added to concept.py . Required tests for qua… #77
Changes from 1 commit
ca218c1
ad78f8d
fb1209a
0270c03
9185513
59ede20
3ca8d30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
3.10.11 | ||
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,5 @@ | ||||||||||||
from enum import Enum | ||||||||||||
from pydantic import BaseModel, Field | ||||||||||||
from pydantic import BaseModel, Field, field_validator | ||||||||||||
from typing import Optional, Dict, Union | ||||||||||||
|
||||||||||||
|
||||||||||||
|
@@ -20,6 +20,22 @@ class Quantity(DataType): | |||||||||||
# TODO: validate conversions str <-> float | ||||||||||||
value: Optional[Union[str, float]] = None | ||||||||||||
unit: Optional[str] = None | ||||||||||||
|
||||||||||||
@field_validator('value') | ||||||||||||
@classmethod | ||||||||||||
def validate_value(cls, value:Union[str,float]): | ||||||||||||
if value is None: | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran the testing suite and got some errors from this line. The logic here should be slightly different. If None is passed in then we want to just return None back, however if a type other than str or float is passed then we can raise the warning. I've added a suggestion below (written on my phone so will need some edits).
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay got it ! Will make the change. |
||||||||||||
raise TypeError(f"Value CANNOT be a {type(value)} object. Must be float or string in float format.") | ||||||||||||
|
||||||||||||
try : | ||||||||||||
return float(value) | ||||||||||||
|
||||||||||||
except ValueError : | ||||||||||||
raise ValueError(f"Invalid value '{value}' . Must be a float Number.") | ||||||||||||
|
||||||||||||
except OverflowError: | ||||||||||||
raise OverflowError(f"Invalid value . Value is too large resulting in overflow.") | ||||||||||||
|
||||||||||||
|
||||||||||||
|
||||||||||||
class Range(DataType): | ||||||||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ termcolor = "^2.4.0" | |
spyne = "^2.14.0" | ||
lxml = "^5.2.2" | ||
xmltodict = "^0.13.0" | ||
pytest = "^8.3.3" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pytest is already in the dev dependencies so doesn't need to be added to the pyproject.toml. (If you check the contribution guide in the repo it has instructions for installing dev dependencies). Can you remove this line and regenerate the poetry.lock? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, by installing the dev dependencies you'll get pre-commit and ruff installed. These are really neat tools that will check your code for some common style errors (number of new lines between functions, max line length) and auto-correct them when you run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. poetry.lock regenerated and dev dependencies installed ! |
||
|
||
[tool.poetry.group.dev.dependencies] | ||
ruff = "^0.4.2" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import pytest | ||
from healthchain.models.data.concept import Quantity | ||
from pydantic import ValidationError | ||
#Valid Cases | ||
def test_valid_float_and_integer(): | ||
valid_floats = [1.0, .1, 4., 5.99999, 12455.321, 33, 1234] | ||
for num in valid_floats : | ||
q = Quantity(value = num,unit = "mg"); | ||
assert q.value == num | ||
|
||
def test_valid_string(): | ||
valid_strings = ["100","100.000001",".1","1.",".123","1234.","123989"] | ||
for string in valid_strings: | ||
q = Quantity(value = string,unit = "mg"); | ||
assert q.value == float(string) | ||
|
||
# Invalid Cases | ||
def test_invalid_strings(): | ||
invalid_strings = ["1.0.0", "1..123", "..123","12..","12a.56","1e4.6","12#.45","12.12@3","12@3"] | ||
for string in invalid_strings: | ||
with pytest.raises(ValidationError) as exception_info: | ||
q = Quantity(value = string,unit = "mg") | ||
assert "Invalid value" in str(exception_info.value) | ||
|
||
|
||
#Edge Cases | ||
def test_edge_cases(): | ||
|
||
edge_cases = ["", "None", None] | ||
for val in edge_cases: | ||
with pytest.raises((ValidationError,TypeError)) as exception_info: | ||
q = Quantity(value = val,unit = "mg") | ||
|
||
exception_info_str = str(exception_info.value) | ||
assert any(msg in exception_info_str for msg in ["CANNOT", "Invalid value"]) | ||
|
||
# if __name__ == '__main__': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you remove the commented out lines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code formatted and commented out lines removed ! |
||
# q = Quantity("12","mg"); | ||
# print(q); |
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.
This file is typically used for local Python version management and shouldn't be tracked in the repository. Could you please unstage this file and add it to your .gitignore? Thanks!
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.
Unstaged ".python-version" and added to .gitignore .