Skip to content

Draft PEP: imath --- module for number-theoretic functions#8

Closed
skirpichev wants to merge 16 commits into
mainfrom
imath
Closed

Draft PEP: imath --- module for number-theoretic functions#8
skirpichev wants to merge 16 commits into
mainfrom
imath

Conversation

@skirpichev
Copy link
Copy Markdown
Owner

No description provided.

Comment thread peps/pep-9999.rst Outdated
Comment thread peps/pep-9999.rst Outdated
@skirpichev
Copy link
Copy Markdown
Owner Author

@vstinner, some major rewrite proposed in #9. Let me know if you more like that version.

Comment thread peps/pep-9999.rst Outdated
Comment thread peps/pep-9999.rst Outdated
Comment thread peps/pep-9999.rst Outdated
Comment thread peps/pep-9999.rst Outdated

Unless we can just provide bindings to some well supported mathematical
library like the GMP, the module scope should be limited. For example, no
primality testing and factorization.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you :-) IMO primality testing is a can a worm and a bad idea for the stdlib :-(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

That was just a first example coming in my mind.

I think it's a good idea to show some border line. For the math/cmath modules we have the C standard to reject new stuff. Here it's less obvious.

Comment thread peps/pep-9999.rst
@vstinner
Copy link
Copy Markdown

@vstinner, some major rewrite proposed in #9. Let me know if you more like that version.

I like this version (PR #8).

@skirpichev
Copy link
Copy Markdown
Owner Author

skirpichev commented May 11, 2025

@NeilGirdhar, per Victor's opinion I'll prefer to defer major rewriting in #9. Some parts, on which it seems we agree - are ported.

If you are ok with this - I'll add you as co-author and create a pr against the peps repo. Or add you to Acknowledgements section.

@NeilGirdhar
Copy link
Copy Markdown

NeilGirdhar commented May 11, 2025

I think the other one reads better (in terms of syntax and grammar, etc.) Since we agree on everything else, why don't we just get rid of the section you don't like in the other one and submit #9?

Comment thread peps/pep-9999.rst
Comment thread peps/pep-9999.rst
Comment on lines +85 to +86
There is already an ``imath`` project on PyPI, but only with two releases, with
the most recent one 4 years ago. Its repository is no longer accessible.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the pypi project may be abandoned, but this Imath is maintained and fairly active.
Naive code search across my employer's repo has hundreds of hits referring to that imath.
could you consider going with a different name please? :)

Copy link
Copy Markdown
Owner Author

@skirpichev skirpichev May 12, 2025

Choose a reason for hiding this comment

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

I appreciate options. Current version is a compromise, based on the poll. As you could see, I'm in a tiny camp of "something else". My initial proposal was "ntheory" (it also has a dead toy project on PyPI).

I would guess, that words like "random" or "fractions" have even more hits. BTW, linked project seems unrelated to the Python ecosystem.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BTW, linked project seems unrelated to the Python ecosystem.

What do you mean by "unrelated"? It's a C++ library with Python bindings!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Hmm, indeed. Though, as naming is still in the "Open Issues", I wouldn't care too much right now.

@vstinner, what do you think if we start from the "ntheory" variant?

@skirpichev
Copy link
Copy Markdown
Owner Author

@NeilGirdhar, I opened python#4422

It looks we fail to reach a consensus on #9, while sponsor seems ok with the current version. So I added you to "Thanks" section so far.

@vstinner
Copy link
Copy Markdown

I suppose that this PR can be closed since python#4422 was created.

@NeilGirdhar
Copy link
Copy Markdown

It looks we fail to reach a consensus on #9, while sponsor seems ok with the current version. So I added you to "Thanks" section so far.

Could you at least fix all of the grammatical errors?

@vstinner
Copy link
Copy Markdown

Could you at least fix all of the grammatical errors?

Would you mind to write a review on python#4422 ?

@skirpichev skirpichev closed this May 12, 2025
@skirpichev skirpichev deleted the imath branch May 12, 2025 10:29
Comment thread peps/pep-9999.rst
Type: Standards Track
Created: 10-May-2025
Python-Version: 3.15
Post-History: 02-Jun-2019,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there no link to the thread?

Comment thread peps/pep-9999.rst
Comment on lines +26 to +27
to the mathematical functions defined by the C standard." But as a state of
art, over time the module was populated with functions that aren't related to
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It should be state of the art

Comment thread peps/pep-9999.rst
The :external+py3.14:mod:`math` documentation says: "This module provides access
to the mathematical functions defined by the C standard." But as a state of
art, over time the module was populated with functions that aren't related to
the C standard or floating-point arithmetics. Now it's much harder to describe
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo, it should be singular.

Comment thread peps/pep-9999.rst
module scope, content and interfaces (returned values or accepted arguments).

For example, the :external+py3.14:mod:`math` module documentation says: "Except
when explicitly noted otherwise, all return values are floats." This is not
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is no longer true...

Comment thread peps/pep-9999.rst
For example, the :external+py3.14:mod:`math` module documentation says: "Except
when explicitly noted otherwise, all return values are floats." This is not
longer true: *None* of the functions listed in the "Number-theoretic
functions" [1]_ subsection of the documentation returns a float, but the
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

IMO it should be return

Comment thread peps/pep-9999.rst
longer true: *None* of the functions listed in the "Number-theoretic
functions" [1]_ subsection of the documentation returns a float, but the
documentation doesn't say so. In the proposed module a similar sentence "All
return values are integers." could tell the truth once. In a similar way we
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How about would consistently reflect reality. The current one is a bit awkward.

Comment thread peps/pep-9999.rst
functions" [1]_ subsection of the documentation returns a float, but the
documentation doesn't say so. In the proposed module a similar sentence "All
return values are integers." could tell the truth once. In a similar way we
can simplify description of accepted arguments for both the
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

simplify the description of accepted arguments

Comment thread peps/pep-9999.rst
can simplify description of accepted arguments for both the
:external+py3.14:mod:`math` and the new module.

Apparently, the :external+py3.14:mod:`math` can't serve as a catch-all place
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the :external+py3.14:mod:math module can't serve

Comment thread peps/pep-9999.rst
:external+py3.14:mod:`math` and the new module.

Apparently, the :external+py3.14:mod:`math` can't serve as a catch-all place
for mathematical functions: we have also the :external+py3.14:mod:`cmath` and
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

we also have the

Comment thread peps/pep-9999.rst

Apparently, the :external+py3.14:mod:`math` can't serve as a catch-all place
for mathematical functions: we have also the :external+py3.14:mod:`cmath` and
the :external+py3.14:mod:`statistics`. Let's make same for integer-related
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's do the same

Comment thread peps/pep-9999.rst
Specification
=============

The PEP proposes moving the following integer-related functions [1]_ in a new
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

functions [1]_ to a new

Sorry I cannot do suggestions, the button is not there and otherwise this is more efficient.

Comment thread peps/pep-9999.rst

Their aliases in :external+py3.14:mod:`math` will be :term:`soft deprecated`.

Modules functions will accept integers and objects that implement the
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Singular, Module

Comment thread peps/pep-9999.rst
:external+py3.14:meth:`~object.__index__` method which is used to convert the
object to an integer number.

Possible extensions for the new module and it's scope are discussed in the
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Possible extensions to the

Comment thread peps/pep-9999.rst
object to an integer number.

Possible extensions for the new module and it's scope are discussed in the
`Open Issues <Open Issues_>`_ section. New functions are not part of the
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

New functions are not part of this Clearer for here IMO

Comment thread peps/pep-9999.rst
Module name
-----------

Chosen name seems consistent with other domain-specific mathematical module:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The chosen name

Comment thread peps/pep-9999.rst
Backwards Compatibility
=======================

As aliases in :external+py3.14:mod:`math` will be kept for indefinite time
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

will be kept for an indefinite time

@serhiy-storchaka
Copy link
Copy Markdown

@StanFromIreland, please use "Start a review" instead of "Add single comment" and publish all your comments at once. The difference is that other people will get one email message instead of an message per comment.

Comment thread peps/pep-9999.rst
Reference Implementation
========================

https://github.com/python/cpython/pull/133909
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should be styled better.

Comment thread peps/pep-9999.rst
Module name
-----------

Chosen name seems consistent with other domain-specific mathematical module:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

modules

Comment thread peps/pep-9999.rst
Chosen name seems consistent with other domain-specific mathematical module:
:external+py3.14:mod:`cmath` (for complex numbers).

There is already an ``imath`` project on PyPI, but only with two releases, with
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we have a link?

Comment thread peps/pep-9999.rst
:external+py3.14:mod:`cmath` (for complex numbers).

There is already an ``imath`` project on PyPI, but only with two releases, with
the most recent one 4 years ago. Its repository is no longer accessible.
Copy link
Copy Markdown

@StanFromIreland StanFromIreland May 12, 2025

Choose a reason for hiding this comment

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

It's

Comment thread peps/pep-9999.rst
There is already an ``imath`` project on PyPI, but only with two releases, with
the most recent one 4 years ago. Its repository is no longer accessible.
The `Imath <https://github.com/AcademySoftwareFoundation/Imath>`_ C++ library
include Python bindings with same name.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

includes

Comment thread peps/pep-9999.rst
There is already an ``imath`` project on PyPI, but only with two releases, with
the most recent one 4 years ago. Its repository is no longer accessible.
The `Imath <https://github.com/AcademySoftwareFoundation/Imath>`_ C++ library
include Python bindings with same name.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

with the same names.

I should have opened pr against this branch instead of my spew of suggestions, apologies.

Comment thread peps/pep-9999.rst
with any PyPI module. On the other hand, ``intmath`` may be confused with
interval math or numerical integration.

Other proposed names include ``ntheory`` (like SymPy's submodule),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe this is better in the past tense now?

Other proposed names included

@vstinner
Copy link
Copy Markdown

@StanFromIreland: You reviewed a closed PR. You should review python#4422 instead.

@StanFromIreland
Copy link
Copy Markdown

Yes, I had not refreshed and missed it… oops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants