Issue37132

Created on **2019-06-02 10:54** by **serhiy.storchaka**, last changed **2020-05-07 16:27** by **mark.dickinson**. This issue is now **closed**.

Pull Requests | |||
---|---|---|---|

URL | Status | Linked | Edit |

PR 13741 | closed | serhiy.storchaka, 2019-06-02 10:58 |

Messages (12) | |||
---|---|---|---|

msg344269 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2019-06-02 10:54 | |

The math module contains as function for floating-point numbers, as wells as functions for integer only numbers: factorial(), gcd(). Yet few integer specific functions was added in 3.8: isqrt(), perm(), comb(). The proposed PR adds the new imath module, adds into it old functions factorial() and gcd() and moves new functions. It also adds two additional functions: as_integer_ratio() and ilog2(). There are plans for adding more integer functions: divide_and_round(), is_prime(), primes(), but the work in progress. |
|||

msg344274 - (view) | Author: Mark Dickinson (mark.dickinson) * | Date: 2019-06-02 11:10 | |

Some questions: - What's the plan for the existing functions `math.gcd` and `math.factorial`? Do they eventually get deprecated, or do we keep `gcd` and `factorial` in both `math` and `imath`? - Does PEP 399 apply here? If so, we'll need a pure Python version as well as a C version - Should `imath.isqrt` be renamed to `imath.sqrt`? - What's the actual benefit of this separation? This feels like a really big change to be making a day before feature freeze; I'm not convinced that we don't need a PEP for this. How bad would it be if we have to defer until 3.9? |
|||

msg344277 - (view) | Author: Mark Dickinson (mark.dickinson) * | Date: 2019-06-02 11:14 | |

> Should `imath.isqrt` be renamed to `imath.sqrt`? On futher reflection, I don't think it should. It's a different function, so it's not like comparing `math.sqrt` and `cmath.sqrt`. One more: I suggest renaming the new function `ilog` to `ilog2`, for consistency with `math.log` versus `math.log2`. |
|||

msg344279 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2019-06-02 11:25 | |

> What's the plan for the existing functions `math.gcd` and `math.factorial`? I think they should eventually get deprecated, but with long term of deprecation. Deprecation warning should be added only when imath versions are already available in older Python versions. But they can be also kept as pure aliases for very long time. > Does PEP 399 apply here? There were no Python implementations when these functions was in the math module. But there are Python implementations for all these functions (used in tests or as predecessors), so it is not hard to add them in the stdlib. Easier that to implement math.sin() on Python from scratch. > Should `imath.isqrt` be renamed to `imath.sqrt`? I considered this idea and do not have answer. > What's the actual benefit of this separation? Currently math becomes a mix of very different functions. Separation will help to keep it simpler (from user and implementation sides). It also adds a place for landing new integer specific functions which are too specific to add them into the math module or into the int class. |
|||

msg344280 - (view) | Author: Mark Dickinson (mark.dickinson) * | Date: 2019-06-02 11:26 | |

I think the new `as_integer_ratio` also needs discussion: it was agreed at some point in the past to add `as_integer_ratio` *methods* on all numeric built-in types, and there's a PR for that. Adding `as_integer_ratio` as a new function too seems like two ways to do it. Please can we defer to 3.9? There just isn't time for the design discussions that are needed, and I'd personally rather see the first version of the `imath` module be somewhat complete, with `is_prime` and `primes` already. I do realise that from the perspective of adding an `imath` module, the accumulation of integer-based functions in the `math` module is something of a wart that we're making worse in 3.8, but I don't think rushing in this change is the solution. Radical suggestion: should we consider delaying the inclusions of `comb`, `perm` and `isqrt` in the math module so that we can do this properly for 3.9? |
|||

msg344281 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2019-06-02 11:39 | |

Actually it was ilog2. There was just an error in the documentation. Currently we have two ways to represent the number as an integer ratio. The official way is two properties numerator and denominator, every number should have them. And some concrete numeric types have the as_integer_ratio() method which returns both values. Rather of adding as_integer_ratio() to all other numeric types I propose to add a helper function which uses either the as_integer_ratio() method if available, or the numerator and denominator properties. Currently any code that uses the as_integer_ratio() method should implement a fallback to numerator and denominator. With imath.as_integer_ratio() it can just use this function. > Radical suggestion: should we consider delaying the inclusions of `comb`, `perm` and `isqrt` in the math module so that we can do this properly for 3.9? I like it. I suggest to delay also adding the int.as_integer_ratio() method. Currently it does not help because there are other numeric methods without the as_integer_ratio() method. |
|||

msg344283 - (view) | Author: Mark Dickinson (mark.dickinson) * | Date: 2019-06-02 12:14 | |

I'm pretty much out of core-dev cycles for this weekend; I'm happy to review GH-13741, but won't have time to do so before next weekend. I'm overall -0 on this change; there's a minor benefit in the separation, but for me it's outweighted by the duplication of `gcd` and `factorial` (or in the case of `gcd`, triplication, since it's also in the `fractions` module). I'm out of time, so I'll be brief, but if this does go into 3.8, here are my thoughts: - please remove the imath.as_integer_ratio function; it needs more discussion, and it has quite a different character from the other functions; it's not at all clear to me that it belongs. We might end up putting it back in eventually, but it needs discussion first, and we don't have time for that discussion before feature freeze. - please add the pure Python version of imath that PEP 399 requires, or get Brett's confirmation that PEP 399 doesn't apply in this case. - please get agreement from one (preferably both) of Tim and Raymond Raymond, Tim: thoughts? |
|||

msg344296 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2019-06-02 16:36 | |

-1 I prefer these functions get left in the regular math module and just organize the docs to separate out integer functions. Additional namespacing creates cognitive load and creates a findability problem. Also, I really don't want gcd() to be moved yet again. |
|||

msg344297 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2019-06-02 16:41 | |

After the beta feature freeze, I'll write up an edit the math module docs that makes in easier to find functions (by splitting out a section of the docs for these functions and by creating a summary link table at the top like we do for builtins). I really don't want a separate module for this and think that would work against the needs of usres. |
|||

msg344381 - (view) | Author: Tim Peters (tim.peters) * | Date: 2019-06-03 04:56 | |

Ya, I'm mostly with Raymond. `math` was originally a very thin wrapper around C's libm, but we've been moving away from that more and more for decades, and it's no longer the case anyway that the vast bulk of new Python programmers are intimately familiar with C. If I were coming new to Python now, I'd expect math functions to ... be in `math` ;-) Whether integer or float. I expect this will work fine if the docs are - as Raymond suggests - reworked to make it clear. |
|||

msg344387 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2019-06-03 07:13 | |

Well, then closing this. |
|||

msg368354 - (view) | Author: Mark Dickinson (mark.dickinson) * | Date: 2020-05-07 16:27 | |

See also #40028 |

History | |||
---|---|---|---|

Date | User | Action | Args |

2020-05-07 16:27:37 | mark.dickinson | set | messages: + msg368354 |

2019-06-03 07:13:59 | serhiy.storchaka | set | status: open -> closed resolution: rejected messages: + msg344387 stage: patch review -> resolved |

2019-06-03 04:56:45 | tim.peters | set | messages: + msg344381 |

2019-06-02 16:41:32 | rhettinger | set | messages: + msg344297 |

2019-06-02 16:36:28 | rhettinger | set | messages: + msg344296 |

2019-06-02 12:14:18 | mark.dickinson | set | messages: + msg344283 |

2019-06-02 11:39:07 | serhiy.storchaka | set | messages: + msg344281 |

2019-06-02 11:26:28 | mark.dickinson | set | messages: + msg344280 |

2019-06-02 11:25:16 | serhiy.storchaka | set | messages: + msg344279 |

2019-06-02 11:14:49 | mark.dickinson | set | messages: + msg344277 |

2019-06-02 11:10:30 | mark.dickinson | set | messages: + msg344274 |

2019-06-02 10:58:49 | serhiy.storchaka | set | keywords:
+ patch stage: patch review pull_requests: + pull_request13624 |

2019-06-02 10:54:57 | serhiy.storchaka | create |