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.

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

Jump to bottom

Doc: Reorganize math module documentation #126337

Merged
merged 14 commits into from
Nov 18, 2024
Merged

Doc: Reorganize math module documentation #126337

merged 14 commits into from
Nov 18, 2024

Conversation

Nodd
Copy link
Contributor

@Nodd Nodd commented Nov 2, 2024 •
edited
Loading

Following #125810, here is a proposal to reorganize the math documentation. I took into account the comments from the previous PRs : #125810 (comment) #125810 (review) #125810 (comment) and tried to improve from here. As suggested in a comment, I used https://en.cppreference.com/w/c/numeric/math as reference.

One thing I'm not fond of is the duplication of the paragraph about the return values, now that frexp and modf are in different sections. Note that modf is also cited in another paragraph in Basic floating point operations.

Another question I have is, should the functions be listed in alphabetic order, or in a logical order ? For example, the trigonometric functions could start with sin and cos instead of acos and asin, or gcd and lcm could be listed together. Same with sqrt and cbrt.

Direct link to the updated math module documentation

📚 Documentation preview 📚: https://cpython-previews--126337.org.readthedocs.build/

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Nov 2, 2024
picnixz
picnixz reviewed Nov 2, 2024
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

cc @skirpichev

Doc/library/math.rst Outdated Show resolved Hide resolved
Doc/library/math.rst Outdated Show resolved Hide resolved
@Nodd
Copy link
Contributor Author

Nodd commented Nov 2, 2024

Thanks @picnixz for the review, I included your modifications.

skirpichev
skirpichev reviewed Nov 3, 2024
View reviewed changes
Doc/library/math.rst Outdated Show resolved Hide resolved
Doc/library/math.rst Show resolved Hide resolved
@skirpichev skirpichev self-requested a review November 3, 2024 05:51
@Nodd Nodd force-pushed the doc_math_organize branch from ac07e54 to a983ae1 Compare November 6, 2024 02:42
@Nodd Nodd marked this pull request as draft November 6, 2024 02:47
@Nodd Nodd marked this pull request as ready for review November 6, 2024 02:55
@Nodd
Copy link
Contributor Author

Nodd commented Nov 6, 2024

Since this PR touches the whole file, merging and fixing the conflict was a pain... If this new layout is fine, can it be merged before another conflict arises? 😅

@vstinner
Copy link
Member

vstinner commented Nov 7, 2024

Overall, I like the new math doc organization. Nice work.

Special functions

I'm not sure how special are these functions. Maybe just say "Other functions"?

@picnixz
Copy link
Member

picnixz commented Nov 7, 2024

We had a brief discussion on this in #126337 (comment) (see also https://mathworld.wolfram.com/SpecialFunction.html).

Now, not everyone knows about the "special functions" terminology which is first informal (there is no formal definition of what a "special function", it's just that we grouped some functions under the category of "Special functions") and second, it allows us to add whatever miscelleanous functions we want to add (although Sergey hinted that we probably won't add more than what the C standard does). So I think "Miscellaneous functions" or "Other functions" is also a good title.

@Nodd
Copy link
Contributor Author

Nodd commented Nov 7, 2024 •
edited
Loading

@vstinner Thank you for your kind words.

About special functions, I proposed a change, see this comment thread. I learned that those are actually special.

@Nodd
Copy link
Contributor Author

Nodd commented Nov 7, 2024

After thinking a bit more about it, one advantage of "Special function" is that it has a meaning to the specialists, and is no less informative than "Miscellaneous functions" or "Other functions" to other people. Those titles are too generic to actually mean anything.

My initial proposal was to be explicit by using "Error and gamma functions". This title could be changed if another function is added in the future. In the end, I simply went with status quo wins.

@skirpichev
Copy link
Member

My initial proposal was to be explicit by using "Error and gamma functions".

This is a best variant, IMO. It's aligned with the C standard.

More generic title (like "Special functions") opens also the door for enhancement requests "lets add also this and that special functions".

@skirpichev skirpichev self-requested a review November 13, 2024 02:04
skirpichev
skirpichev reviewed Nov 14, 2024
View reviewed changes
Doc/library/math.rst Outdated Show resolved Hide resolved
picnixz
picnixz approved these changes Nov 14, 2024
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

LGTM (modulo Sergey's comment).

@vstinner vstinner merged commit ce453e6 into python:main Nov 18, 2024
25 checks passed
@vstinner
Copy link
Member

Merged, thank you.

Should we backport the change to avoid merge conflicts?

@skirpichev
Copy link
Member

Should we backport the change to avoid merge conflicts?

Yes, I think so. Because #125810 was backported.

@vstinner vstinner added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Nov 18, 2024
@miss-islington-app
Copy link

Thanks @Nodd for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @Nodd for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @Nodd and @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker ce453e6c2ffda657d9d728ea6372121e8264418e 3.12

@miss-islington-app
Copy link

Sorry, @Nodd and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker ce453e6c2ffda657d9d728ea6372121e8264418e 3.13

@vstinner
Copy link
Member

@Nodd: Would you mind to try to backport this change to the 3.13 branch manually? There are merge conflicts.

@skirpichev
Copy link
Member

3.12 backport also fails. Its all coming from #126215.

@Nodd
Copy link
Contributor Author

Nodd commented Nov 18, 2024

Weird, I thought I already rebased the PR after the other modification. I'll look into it.

@vstinner
Copy link
Member

Weird, I thought I already rebased the PR after the other modification. I'll look into it.

The change is merged into the main branch. I'm asking about backporting the change to the 3.13 branch. Maybe some other doc changes were not backported to 3.13?

skirpichev added a commit to skirpichev/cpython that referenced this pull request Nov 19, 2024
@Nodd @picnixz @skirpichev
[3.13] Doc: Reorganize math module documentation (pythonGH-126337)
1f9f096 
(cherry picked from commit ce453e6)

Co-authored-by: Joseph Martinot-Lagarde <contrebasse@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Nov 19, 2024

GH-126998 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 19, 2024
skirpichev added a commit to skirpichev/cpython that referenced this pull request Nov 19, 2024
@Nodd @picnixz @skirpichev
[3.13] Doc: Reorganize math module documentation (pythonGH-126337)
bdfbb51 
(cherry picked from commit ce453e6)

Co-authored-by: Joseph Martinot-Lagarde <contrebasse@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@skirpichev
Copy link
Member

I took liberty to do backport for 3.13.

The 3.12 backport blocked by #126309.

vstinner pushed a commit that referenced this pull request Nov 19, 2024
@skirpichev @Nodd @picnixz
[3.13] Doc: Reorganize math module documentation (GH-126337) (#126998)
190d710 
(cherry picked from commit ce453e6)

Co-authored-by: Joseph Martinot-Lagarde <contrebasse@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@vstinner vstinner mentioned this pull request Nov 19, 2024
Closed
@Nodd
Copy link
Contributor Author

Nodd commented Nov 20, 2024

Thank you @skirpichev for the backport.

ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@Nodd @picnixz
@skirpichev @ebonnal
Doc: Reorganize math module documentation (python#126337)
7767af3 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@picnixz picnixz picnixz approved these changes

@skirpichev skirpichev skirpichev approved these changes

@vstinner vstinner vstinner approved these changes

Assignees

@vstinner vstinner

Labels
docs Documentation in the Doc dir needs backport to 3.12 bug and security fixes skip issue skip news
Projects
Docs PRs
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Nodd @vstinner @picnixz @skirpichev