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

Respect case of xsi:double infinity and NaN #338

Merged
merged 2 commits into from Feb 13, 2022

Conversation

haxtibal
Copy link
Contributor

See lxml launchpad bug 1960715.

I'm not too experienced with Cython. Feel free to edit the commit.

src/lxml/objectify.pyx Outdated Show resolved Hide resolved
@scoder
Copy link
Member

scoder commented Feb 12, 2022

Thanks. Could you please add tests in src/lxml/tests/test_objectify.py? It looks like we only test these values as input, not as output or type markers.

src/lxml/objectify.pyx Outdated Show resolved Hide resolved
src/lxml/objectify.pyx Outdated Show resolved Hide resolved
src/lxml/objectify.pyx Outdated Show resolved Hide resolved
@haxtibal
Copy link
Contributor Author

Could you please add tests in src/lxml/tests/test_objectify.py?

Added some tests. Not sure if they're complete enough or in the right place, would you have a look?

Copy link
Member

@scoder scoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good.

src/lxml/objectify.pyx Outdated Show resolved Hide resolved
W3C specification for xsd:double says
> The special values positive and negative infinity and
> not-a-number have lexical representations INF, -INF and NaN,
> respectively.

Thus case matters. The previously used float.__repr__ would generate
"inf", "-inf", "nan". Now we prepend special handling to get
"INF", "-INF", "NaN" instead (which is still pytype compatible).

Includes minor non-functional alignments of related bool to text code,
and tests to assert its XML schema conformance as well.
@haxtibal
Copy link
Contributor Author

Thanks for your review.

Recent force push was merely to align author and committer email in my commit. They differed as a result of bad git project settings on my side.

@scoder scoder merged commit f7bb07b into lxml:master Feb 13, 2022
@scoder
Copy link
Member

scoder commented Feb 13, 2022

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants