GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-102837: few coverage nitpicks for the math module by skirpichev · Pull Request #102523 · python/cpython

Conversation

@skirpichev

@skirpichev skirpichev commented Mar 8, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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)

  • Issue: Improve test coverage for the math module #102837

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added the awaiting review label Mar 8, 2023
@skirpichev

skirpichev commented Mar 8, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood AlexWaygood added the skip news label Mar 8, 2023
@skirpichev

skirpichev commented Mar 8, 2023

Copy link
Copy Markdown
Member Author

@AlexWaygood, do you think this requires an issue?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood

AlexWaygood commented Mar 8, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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 :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@skirpichev

skirpichev commented Mar 8, 2023 via email

Copy link
Copy Markdown
Member Author
On Wed, Mar 08, 2023 at 06:21:43AM -0800, Alex Waygood wrote: Our policy is generally to require issues for all changes, unless they're extremely trivial (such as typo fixes).
Ok. I thought this is trivial:)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mdickinson
mdickinson self-requested a review March 8, 2023 17:40
@skirpichev
pythongh-102837: few coverage nitpicks for the math module
68b2e42 
- 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)
@skirpichev
skirpichev force-pushed the math-cov branch from 3e3807b to 68b2e42 Compare March 20, 2023 07:26
@skirpichev skirpichev changed the title Few coverage nitpicks for the math module gh-102837: few coverage nitpicks for the math module Mar 20, 2023
@bedevere-bot bedevere-bot mentioned this pull request Mar 20, 2023
Improve test coverage for the math module #102837
Closed
skirpichev added 9 commits April 3, 2023 14:22
@skirpichev
Merge branch 'main' into math-cov
dc85378
@skirpichev
Merge branch 'main' into math-cov
86aee3c
@skirpichev
Simplify math_1, like other functions
9c32ae0 
Now exception messages kept in is_error().  This improve
coverage for L1007.
@skirpichev
Merge branch 'main' into math-cov
b8cf8ac
@skirpichev
Merge branch 'main' into math-cov
b395cfb
@skirpichev
Merge branch 'main' into math-cov
c3bd1cd
@skirpichev
Merge branch 'main' into math-cov
dc8ac10
@skirpichev
Merge branch 'main' into math-cov
acb851d
@skirpichev
Merge branch 'main' into math-cov
a6c769e
hauntsaninja
hauntsaninja reviewed Sep 1, 2023
View reviewed changes

@hauntsaninja hauntsaninja left a comment

Copy link
Copy Markdown
Contributor

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.

Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
skirpichev added 3 commits September 2, 2023 05:14
@skirpichev
Merge branch 'math-cov' of github.com:skirpichev/cpython into math-cov
d45ea20
@skirpichev
Merge branch 'main' into math-cov
59a5422
@skirpichev
Add asserts
a0c619b
@skirpichev
skirpichev requested a review from hauntsaninja September 2, 2023 02:36
@skirpichev
skirpichev force-pushed the math-cov branch from 6634b88 to a0c619b Compare September 2, 2023 03:24
@skirpichev
Amend 9c32ae0 (comment restored)
b991b7c
hauntsaninja
hauntsaninja reviewed Sep 2, 2023
View reviewed changes
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
skirpichev added 2 commits September 2, 2023 09:15
@skirpichev
Merge branch 'main' into math-cov
192a279
@skirpichev
Revert back math_floor(), add tests to cover L1239 (same for ceil())
828ba75
@skirpichev
Tests for bad __floor__/__ceil__ descriptors (cover L1165, L1236)
6498162
@skirpichev

skirpichev commented Sep 2, 2023

Copy link
Copy Markdown
Member Author

Last commit will cover rest of math_floor()/ceil().
hauntsaninja reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
skirpichev added 3 commits September 3, 2023 06:42
@skirpichev
+1
8fcca16
@skirpichev
Revert "Simplify math_1, like other functions"
796bdca 
This reverts commit 9c32ae0.
@skirpichev
Merge branch 'main' into math-cov
c7d1764
@skirpichev
skirpichev requested a review from hauntsaninja September 3, 2023 04:29
hauntsaninja
hauntsaninja approved these changes Sep 3, 2023
View reviewed changes

@hauntsaninja hauntsaninja left a comment

Copy link
Copy Markdown
Contributor

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.

Thanks for the follow ups! :-)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Sep 3, 2023
@hauntsaninja
hauntsaninja merged commit 9c995ab into python:main Sep 3, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Sep 3, 2023
@skirpichev
skirpichev deleted the math-cov branch September 3, 2023 09:03
@skirpichev skirpichev mentioned this pull request Sep 3, 2023
gh-110489: Optimise math.ceil for known exact float #108801
Merged
This was referenced Sep 26, 2023
gh-101410: Improve error messages in math module #101492
Closed
gh-102837: Increase test coverage for the math module #110000
Closed
@skirpichev

skirpichev commented Sep 28, 2023

Copy link
Copy Markdown
Member Author

Next pr: #110000
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Oct 6, 2023
View reviewed changes
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/mathmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
skirpichev added a commit to skirpichev/cpython that referenced this pull request Oct 6, 2023
@skirpichev
More efficient (for finite x) handling of special cases in math.modf
3df127e 
Rewrite changes in python#102523
@skirpichev skirpichev mentioned this pull request Nov 13, 2023
[3.12] gh-102837: more tests for the math module (GH-111930) #112029
Closed
encukou pushed a commit to encukou/cpython that referenced this pull request Nov 13, 2023
@skirpichev @encukou
pythongh-102837: improve test coverage for math module (python#102523) (
42d1d23 
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

bedevere-app Bot commented Nov 13, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou added a commit that referenced this pull request Nov 13, 2023
@encukou @skirpichev
[3.12] gh-102837: more tests for the math module (GH-111930)(GH-102523)…
c6aea46 
… (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 <[email protected]>
@encukou encukou mentioned this pull request Nov 13, 2023
[3.11] gh-102837: more tests for the math module (GH-111930)(GH-102523) (GH-112030) #112041
Merged
encukou added a commit that referenced this pull request Nov 16, 2023
@encukou @skirpichev
[3.11] gh-102837: more tests for the math module (GH-111930)(GH-102523)…
132a749 
… (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 <[email protected]>
(cherry picked from commit c6aea46)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@hauntsaninja hauntsaninja hauntsaninja approved these changes
@mdickinson mdickinson Awaiting requested review from mdickinson

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@skirpichev @AlexWaygood @serhiy-storchaka @hauntsaninja @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.