-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
gh-81313: Add the math.integer module (PEP-791) #133909
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.
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
Conversation
|
This is a further development of #13741. |
|
A PEP is being written for this module: skirpichev/peps#8 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
No, this would only make things worse. This would break the module name for The right solution is to compile the math module to |
|
I used other way to fix the module name. This is a temporary hack, I hope we will make it a straight way in future. |
|
I think test failures are relevant. |
Misc/NEWS.d/next/Library/2019-06-02-13-56-16.gh-issue-81313.axawSH.rst
Outdated
Show resolved
Hide resolved
IMO it's good enough for a first iteration, we will have enough time until Python 3.15 final to find a way to fix these issues. |
…awSH.rst Co-authored-by: Victor Stinner <vstinner@python.org>
skirpichev
left a comment
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.
Playing with the new module, I found also this:
>>> math.integer.__spec__
ModuleSpec(name='_math_integer', loader=<_frozen_importlib_external.ExtensionFileLoader object at 0x7fdbdae6f150>, origin='/home/sk/src/cpython/build/lib.linux-x86_64-3.15/_math_integer.cpython-315-x86_64-linux-gnu.so')| For backward compatibility, the :mod:`math` module provides also aliases of | ||
| the following functions from the :mod:`math.integer` module: |
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.
Please mention there, that these aliases are soft-deprecated.
BTW, why this section was moved?
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.
Because it is less important than other sections.
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.
Maybe it is, if we want to communicate users this renaming. Though, it's important only for those who read docs in order... Up to you.
| /* Fix the __name__ attribute of the module and the __module__ attribute | ||
| * of its functions. | ||
| */ |
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.
Could you, please, create an issue to track this?
IIUIC, currently it's also not clear how to name the module file (math_integermodule.c, math/integermodule.c or even intmathmodule.c). I.e. we should extend current naming scheme to submodules.
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.
See #140824.
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
skirpichev
left a comment
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.
LGTM, with few nitpicks (up to Serhiy) and note about ModuleSpec(). (I suspect this might affect something.)
vstinner
left a comment
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.
LGTM
| .. versionchanged:: 3.10 | ||
| Floats with integral values (like ``5.0``) are no longer accepted in the | ||
| :func:`factorial` function. | ||
|
|
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.
Maybe add a versionchanged to mention that these functions became aliases to math.integer functions?
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.
This did not change their behavior.
| Floats with integral values (like ``5.0``) are no longer accepted in the | ||
| :func:`factorial` function. | ||
|
|
||
|
|
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.
You should add .. deprecated:: 3.15 to repeat that these functions are soft deprecated and math.integer functions should be used instead.
📚 Documentation preview 📚: https://cpython-previews--133909.org.readthedocs.build/