X Tutup
Skip to content
/ lxml Public

Respect case of xsi:double infinity and NaN#338

Merged
scoder merged 2 commits intolxml:masterfrom
haxtibal:bugfix/xsi_double_INF
Feb 13, 2022
Merged

Respect case of xsi:double infinity and NaN#338
scoder merged 2 commits intolxml:masterfrom
haxtibal:bugfix/xsi_double_INF

Conversation

@haxtibal
Copy link
Contributor

See lxml launchpad bug 1960715.

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

@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.

@haxtibal haxtibal force-pushed the bugfix/xsi_double_INF branch from f09e8fa to 9fb37df Compare February 12, 2022 22:50
@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.

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 haxtibal force-pushed the bugfix/xsi_double_INF branch from e6eca3b to 82efad3 Compare February 13, 2022 07:41
@haxtibal haxtibal force-pushed the bugfix/xsi_double_INF branch from 82efad3 to 480daf1 Compare February 13, 2022 07:44
@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

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup