Skip to content

gh-102837: few coverage nitpicks for the math module - #102523

Merged
hauntsaninja merged 20 commits into
python:mainfrom
skirpichev:math-cov
Sep 3, 2023
Merged

gh-102837: few coverage nitpicks for the math module#102523
hauntsaninja merged 20 commits into
python:mainfrom
skirpichev:math-cov

Conversation

@skirpichev

@skirpichevskirpichev commented Mar 8, 2023

Copy link
Copy Markdown
Member
  • input checks for math_1(L989), math_1a(L1023), math_2(L1064,L1071), hypot(L2682), log(L2307), ldexp(L2168), ceil(L1165), floor(L1236,L1239) and dist(L2587,L2588,L2628).
  • drop inaccessible "if" branch (L3518) in perm_comb_small()
  • improve fsum coverage for exceptional cases (L1433,L1438,L1451,L1497), ditto fmod(L2378)
  • rewrite modf to fix inaccessible case(L2229), ditto for pow(L2988)

(all line numbers are wrt the main branch at 5e6661b)

@skirpichev

skirpichev commented Mar 8, 2023

Copy link
Copy Markdown
MemberAuthor

Note, c.f. #102067 for the cmath module - there are still some uncovered lines/branches.

@skirpichev

Copy link
Copy Markdown
MemberAuthor

@AlexWaygood, do you think this requires an issue?

@AlexWaygood

AlexWaygood commented Mar 8, 2023

Copy link
Copy Markdown
Member

@AlexWaygood, do you think this requires an issue?

Our policy is generally to require issues for all changes, unless they're extremely trivial (such as typo fixes).

(If the maintainers of the math module decide this doesn't need an issue then that's obviously fine, but I'm not a math module maintainer, so I don't want to decide that for them :)

@skirpichev

skirpichev commented Mar 8, 2023 via email

Copy link
Copy Markdown
MemberAuthor

@mdickinson
mdickinson self-requested a review March 8, 2023 17:40
- input checks for math_1(L989), math_1a(L1023), math_2(L1064,L1071),
hypot(L2682), log(L2307), ldexp(L2168) and dist(L2587,L2588,L2628).
- rewrite math_floor like math_ceil (cover L1239)
- drop inaccessible "if" branch (L3518) in perm_comb_small()
- improve fsum coverage for exceptional cases (L1433,L1438,L1451,L1497),
ditto fmod(L2378)
- rewrite modf to fix inaccessible case(L2229), ditto for pow(L2988)
(all line numbers wrt the main branch at 5e6661b)
@skirpichevskirpichev changed the title Few coverage nitpicks for the math modulegh-102837: few coverage nitpicks for the math moduleMar 20, 2023

@hauntsaninjahauntsaninja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

Comment threadModules/mathmodule.c
Comment threadModules/mathmodule.c Outdated
Comment threadModules/mathmodule.c Outdated
Comment threadModules/mathmodule.c Outdated
Comment threadModules/mathmodule.c Outdated
@skirpichev

Copy link
Copy Markdown
MemberAuthor

Last commit will cover rest of math_floor()/ceil().

@hauntsaninjahauntsaninja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the follow ups! :-)

@skirpichev

Copy link
Copy Markdown
MemberAuthor

Next pr: #110000

Comment threadModules/mathmodule.c
Comment threadModules/mathmodule.c
skirpichev added a commit to skirpichev/cpython that referenced this pull request Oct 6, 2023
encukou pushed a commit to encukou/cpython that referenced this pull request Nov 13, 2023
pythonGH-102523)
(Only the test changes from pythonGH-102523 are cherry-picked)
- input checks for math_1(L989), math_1a(L1023), math_2(L1064,L1071), hypot(L2682), log(L2307), ldexp(L2168), ceil(L1165), floor(L1236,L1239) and dist(L2587,L2588,L2628).
- improve fsum coverage for exceptional cases (L1433,L1438,L1451,L1497), ditto fmod(L2378)
(all line numbers are wrt the main branch at 5e6661b)
@bedevere-app

Copy link
Copy Markdown

GH-112030 is a backport of this pull request to the 3.12 branch.

encukou added a commit that referenced this pull request Nov 13, 2023
… (GH-112030)
* gh-102837: improve test coverage for math module (GH-102523)
(Only the test changes from GH-102523 are cherry-picked)
- input checks for math_1(L989), math_1a(L1023), math_2(L1064,L1071), hypot(L2682), log(L2307), ldexp(L2168), ceil(L1165), floor(L1236,L1239) and dist(L2587,L2588,L2628).
- improve fsum coverage for exceptional cases (L1433,L1438,L1451,L1497), ditto fmod(L2378)
(all line numbers are wrt the main branch at 5e6661b)
* gh-102837: more tests for the math module (GH-111930)
Add tests to improve coverage:
* fsum: L1369, L1379, L1383, L1412
* trunc: L2081
* log: L2267
* dist: L2577, L2579
* hypot: L2632
* sumprod: L2744, L2754, L2774, L2778, L2781, L2785, L2831, L2835, L2838
* pow: L2982
* prod: L3294, L3308, L3318-3330
// line numbers wrt to 9dc4fb8
(cherry picked from commit c61de45)
---------
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
encukou added a commit that referenced this pull request Nov 16, 2023
… (GH-112030) (GH-112041)
[3.12] gh-102837: more tests for the math module (GH-111930)(GH-102523) (GH-112030)
* gh-102837: improve test coverage for math module (GH-102523)
(Only the test changes from GH-102523 are cherry-picked)
- input checks for math_1(L989), math_1a(L1023), math_2(L1064,L1071), hypot(L2682), log(L2307), ldexp(L2168), ceil(L1165), floor(L1236,L1239) and dist(L2587,L2588,L2628).
- improve fsum coverage for exceptional cases (L1433,L1438,L1451,L1497), ditto fmod(L2378)
(all line numbers are wrt the main branch at 5e6661b)
* gh-102837: more tests for the math module (GH-111930)
Add tests to improve coverage:
* fsum: L1369, L1379, L1383, L1412
* trunc: L2081
* log: L2267
* dist: L2577, L2579
* hypot: L2632
* (not cherry-picked for 3.11: sumprod)
* pow: L2982
* prod: L3294, L3308, L3318-3330
// line numbers wrt to 9dc4fb8
(cherry picked from commit c61de45)
---------
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
(cherry picked from commit c6aea46)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@skirpichev@AlexWaygood@serhiy-storchaka@hauntsaninja@bedevere-bot