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.

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

Provide as_integer_ratio() method to Decimal #53193

Closed
abalkin opened this issue Jun 8, 2010 · 11 comments
Closed

Provide as_integer_ratio() method to Decimal #53193

abalkin opened this issue Jun 8, 2010 · 11 comments
Assignees
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@abalkin
Copy link
Member

abalkin commented Jun 8, 2010

BPO 8947
Nosy @rhettinger, @mdickinson, @abalkin, @skrah
Superseder
  • bpo-25928: Add Decimal.as_integer_ratio()
  • Files
  • issue8947.patch
  • issue8947_2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/mdickinson'
    closed_at = <Date 2010-06-29.20:07:25.985>
    created_at = <Date 2010-06-08.20:51:06.757>
    labels = ['extension-modules', 'type-feature']
    title = 'Provide as_integer_ratio() method to Decimal'
    updated_at = <Date 2018-03-13.19:04:17.701>
    user = 'https://github.com/abalkin'

    bugs.python.org fields:

    activity = <Date 2018-03-13.19:04:17.701>
    actor = 'belopolsky'
    assignee = 'mark.dickinson'
    closed = True
    closed_date = <Date 2010-06-29.20:07:25.985>
    closer = 'mark.dickinson'
    components = ['Extension Modules']
    creation = <Date 2010-06-08.20:51:06.757>
    creator = 'belopolsky'
    dependencies = []
    files = ['17626', '17627']
    hgrepos = []
    issue_num = 8947
    keywords = ['patch']
    message_count = 11.0
    messages = ['107345', '107348', '107542', '107543', '107622', '108600', '108613', '108615', '108944', '313767', '313768']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'mark.dickinson', 'belopolsky', 'skrah']
    pr_nums = []
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '25928'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue8947'
    versions = ['Python 3.2']

    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 8, 2010

    Here is my use case: recently implemented timedelta * float operation starts with converting float to an int ratio. The same algorithm can be used for decimals, but they don't support as_integer_ratio().

    @abalkin abalkin added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Jun 8, 2010
    @mdickinson
    Copy link
    Member

    This seems like a reasonable request to me; I'd like to see a little bit of convergence between the float and Decimal APIs.

    One difference between the two types that's worth noting is that the float.as_integer_ratio() method can never take a ridiculous amount of time or memory, whereas Decimal.as_integer_ratio() could: e.g., consider Decimal('1e999999999').as_integer_ratio(). But I don't really see that as a problem.

    @mdickinson mdickinson self-assigned this Jun 8, 2010
    @mdickinson
    Copy link
    Member

    Here's a patch.

    @mdickinson
    Copy link
    Member

    Updated patch, taking into account comments from merwok and exarkun on #python-dev:

    • remove doctests for infinity and nan, replace with a sentence
      explaining what happens for such inputs.
    • replace 'snAN' with saner spelling 'snan'.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 12, 2010

    Nice implementation. I wonder if the /5 loop can be eliminated by noting that /5 is *2 followed by decimal shift. (Probably not without fast decimal arithmetics.)

    A few documentation nits:

    1. In Decimal methods there is no consistency in referring to self. I see "given Decimal", "the number", and "the argument" in three entries around as_integer_ratio().

    2. Is there a reason that docstring is more detailed than manual entry? I think the manual should describe exceptions.

    3. Is there a reason to use different language for float.as_integer_ratio() and Decimal.as_integer_ratio()?

    I know, ""A foolish consistency is the hobgoblin of little minds..." - feel free to ignore my observations.

    A tiny code nit: to me it would be clearer to start with d2 = d5 = -self._exp after the "# Find d2, d5 ..." comment. For a moment I was puzzled why you promise d2 and d5, but then process d5 only.

    Also, by the time of the "if not self" check, special case has been eliminated, so you can simply check self._int == 0.

    @mdickinson
    Copy link
    Member

    Raymond, do you have any thoughts on this proposal?

    @rhettinger
    Copy link
    Contributor

    -0 I don't think we really need this (we've already got a reasonable conversion to Fraction).

    In general, we should have a aversion to adding any methods to the decimal API because it is already very fat. Adding more methods makes it harder to figure out which one is the right one to use.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 25, 2010

    Raymond,

    conversion to Fraction does not really help in my use case of supporting timedelta * Decimal by duck typing. I can think of many other use cases where it will be helpful to accept either float or decimal without loss of precision. Is there an alternative way to achieve that which does not require importing the decimal module first?

    @mdickinson
    Copy link
    Member

    After discussions on IRC, I'm going to close this as rejected. Convergence of the float and Decimal APIs is a nice idea, but in this case it's not clear that it really works. In particular, for duck typing to be sensible, int and Fraction would also have to grow an as_integer_ratio method. And that seems a superfluous when those two types already have the same functionality in the form of .numerator and .denominator attributes.

    I also agree with Raymond that we shouldn't be fattening the decimal API without good reason.

    Alexander: wouldn't conversion to Fraction solve the issue in this case? The Fraction constructor accepts floats, Decimal instances and ints directly.

    @mdickinson mdickinson changed the title Provide as_integer_ratio() method to Decimal as_integer_ratio Decimal Mar 13, 2018
    @mdickinson mdickinson changed the title as_integer_ratio Decimal Provide as_integer_ratio() method to Decimal Mar 13, 2018
    @mdickinson
    Copy link
    Member

    Apologies: the title change was accidental. Reverted.

    @abalkin
    Copy link
    Member Author

    abalkin commented Mar 13, 2018

    This issue has been superseded by bpo-25928.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants