-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix bug #264
fix bug #264
Conversation
@@ -210,3 +210,14 @@ def test_get_attributes_from_person(): | |||
assert pid == "chris" | |||
assert attributes["hid"] == "A" | |||
assert attributes["vehicles"] == {"car": "chris"} | |||
|
|||
|
|||
def test_get_float_attribute_from_person(): |
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.
When you ran this test before fixing the bug, did it throw an exception?
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.
yep
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.
it was trying to cast a lxml element as float (float(attr)
), instead of the element value (attr.text
)
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 catch!
@Theodore-Chatziioannou you need to run pre-commit locally! |
|
hmm, pre-commit should have fixed this issue when you first committed (assuming you have run |
Fix for #263 .
Checklist
Any checks which are not relevant to the PR can be pre-checked by the PR creator.
All others should be checked by the reviewer(s).
You can add extra checklist items here if required by the PR.