Skip to content

gh-148868: Increase test coverage for cmath.isinf #148869

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.

Sign up for GitHub

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

Open
manoj-km24 wants to merge 2 commits into
base: main
Choose a base branch
from
+1 −0
Open

gh-148868: Increase test coverage for cmath.isinf #148869

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5495f7
Add additional edge case tests for cmath.py
manoj-km24 Apr 22, 2026
08187ca
Remove INF/INF case
manoj-km24 Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Lib/test/test_cmath.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,8 @@ def test_isinf(self):
self.assertFalse(cmath.isinf(1j))
self.assertFalse(cmath.isinf(NAN))
self.assertTrue(cmath.isinf(INF))
self.assertTrue(cmath.isinf(-INF))
self.assertFalse(cmath.isinf(INF/INF))
Copy link
Copy Markdown
Member

@picnixz picnixz Apr 22, 2026

Choose a reason for hiding this comment

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

Why this? INF/INF is NaN so it does not make any sense

Copy link
Copy Markdown
Contributor Author

@manoj-km24 manoj-km24 Apr 22, 2026

Choose a reason for hiding this comment

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

INF/INF seems to be an exception because when we take INF + INF and INF * INF , they produce INF but on the other hand INF/INF produces NaN. So I thought it would be worth testing that too . I could change it if you like

Copy link
Copy Markdown
Member

@picnixz picnixz Apr 22, 2026

Choose a reason for hiding this comment

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

It is not an exception, it is what the standard expects (and what maths tells you about). Adding two infinities or multiplying result in an infinity. However division and subtration of infinities do not have a limit hence a NaN. Please make sure you understand the issue before creating one and opening a PR in the future.

So, remove this case. You can keep the -INF one but I do not really think it is worth. In particular we want to check that isinf from math (and not cmath) also checks negative infinity.

Copy link
Copy Markdown
Contributor Author

@manoj-km24 manoj-km24 Apr 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Will do . Sorry , i misused the word "exception". What I wanted to say was if in case we change the behaviour of INF, would it be better if we check what subtracting or dividing 2 infinities produces , as the standard expects since addition and multiplication of 2 infinities are obvious. Just a doubt ,I will clear the INF/INF case in a minute.

self.assertTrue(cmath.isinf(complex(INF, 0)))
self.assertTrue(cmath.isinf(complex(0, INF)))
self.assertTrue(cmath.isinf(complex(INF, INF)))
Expand Down
Loading