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

float() can return not exact float instance #71170

Closed
serhiy-storchaka opened this issue May 9, 2016 · 15 comments
Closed

float() can return not exact float instance #71170

serhiy-storchaka opened this issue May 9, 2016 · 15 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

BPO 26983
Nosy @mdickinson, @serhiy-storchaka
Dependencies
  • bpo-26995: Add tests for parsing float and object arguments
  • Files
  • float_exact.patch
  • float_exact2.patch
  • issue26983.diff
  • 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/serhiy-storchaka'
    closed_at = <Date 2016-06-06.10:01:54.560>
    created_at = <Date 2016-05-09.13:46:33.061>
    labels = ['interpreter-core', 'type-bug']
    title = 'float() can return not exact float instance'
    updated_at = <Date 2016-06-06.10:01:54.558>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2016-06-06.10:01:54.558>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2016-06-06.10:01:54.560>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2016-05-09.13:46:33.061>
    creator = 'serhiy.storchaka'
    dependencies = ['26995']
    files = ['42807', '42815', '43256']
    hgrepos = []
    issue_num = 26983
    keywords = ['patch']
    message_count = 15.0
    messages = ['265191', '265267', '265295', '265297', '265298', '265299', '265316', '265318', '267096', '267101', '267129', '267130', '267521', '267523', '267524']
    nosy_count = 4.0
    nosy_names = ['mark.dickinson', 'SilentGhost', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'commit review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue26983'
    versions = ['Python 3.6']

    @serhiy-storchaka
    Copy link
    Member Author

    The float constructor can return an instance of float subclass.

    >>> class FloatSubclass(float):
    ...     pass
    ... 
    >>> class BadFloat:
    ...     def __float__(self):
    ...         return FloatSubclass(1.2)
    ... 
    >>> type(float(BadFloat()))
    <class '__main__.FloatSubclass'>

    Comparing with other types, complex() always returns complex:

    >>> class ComplexSubclass(complex):
    ...     pass
    ... 
    >>> class BadComplex:
    ...     def __complex__(self):
    ...         return ComplexSubclass(1.2, 3.4)
    ... 
    >>> type(complex(BadComplex()))
    <class 'complex'>

    And int() can return an instance of int subclass, but this behavior is deprecated:

    >>> class BadInt:
    ...     def __int__(self):
    ...         return True
    ... 
    >>> int(BadInt())
    __main__:1: DeprecationWarning: __int__ returned non-int (type bool).  The ability to return an instance of a strict subclass of int is deprecated, and may be removed in a future version of Python.
    True

    May be we should either deprecate __float__ returning non-float (as for int), or convert the result to exact float (as for complex).

    The constructor of float subclass always returns an instance of correct type.

    >>> class FloatSubclass2(float):
    ...     pass
    ... 
    >>> type(FloatSubclass2(BadFloat()))
    <class '__main__.FloatSubclass2'>

    @serhiy-storchaka serhiy-storchaka added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels May 9, 2016
    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patch makes float() always returning exact float.

    @mdickinson
    Copy link
    Member

    +1 for the idea, and the patch LGTM.

    I was initially a bit confused by the wording of the warning: presumably, we're not going to change Python to make returning an instance of a strict float subclass from __float__ illegal (I don't really see how that would be possible); we're just going to check the return type at the internal call sites to __float__ and raise if we get something other than an exact float. That is, I'd still expect this code to work on future versions of Python:

    >>> class MyFloat(float): pass
    ... 
    >>> class A:
    ...     def __float__(self): return MyFloat()
    ... 
    >>> a = A()
    >>> a.__float__()
    0.0

    But these would both become an error in Python 3.7 (say):

    >>> float(a)
    0.0
    >>> math.sqrt(a)
    0.0

    Does that match your thinking?

    We should probably add issues for checking and fixing other places that __float__ is used internally (like the math module).

    @serhiy-storchaka
    Copy link
    Member Author

    The warning message is copied from __int__. As I understand, the conclusion of the Python-Dev discussion is that __int__, __float__, __complex__, etc should return exact int, float, complex, not subclasses. I have another patch that adds a warning for __complex__, and I am working on patches for other special methods. If my understanding is wrong, we should remove the deprecation warning for __int__.

    @mdickinson
    Copy link
    Member

    As I understand, the conclusion of the Python-Dev discussion is that __int__, __float__, __complex__, etc should return exact int, float, complex, not subclasses.

    Agreed; that matches my understanding. I'm only observing that it would be difficult (and likely undesirable) to actually enforce that __int__ itself always returns an exact int: I'm not sure where that check would/could take place, and I'd expect that a direct user-level call to my_object.__int__ probably wouldn't be subject to such a check. What we can do is check that the result of the nb_int call is always an int in the places we use it.

    Or we could even go further, and write custom slot_nb_float and slot_nb_int functions in Objects/typeobject.c that incorporates those checks. What I don't think we can do is check that a direct call to __int__ or __float__ always returns something of the exact type.

    @serhiy-storchaka
    Copy link
    Member Author

    Here is updated patch. Differences:

    • Deprecation warning is now emitted in functions accepting floats (like math.sqrt).
    • Improved error messages. Now reported wrong type name and a type of wrong __float__ method.
    • float() now returns the same object for exact float argument.

    Could you propose better warning message?

    @mdickinson
    Copy link
    Member

    Can you please attach the new patch?

    @serhiy-storchaka
    Copy link
    Member Author

    Oh, sorry, how could I miss it?

    @serhiy-storchaka
    Copy link
    Member Author

    Ping.

    @mdickinson
    Copy link
    Member

    float_exact2.patch LGTM

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 3, 2016

    New changeset 050e5f803999 by Serhiy Storchaka in branch 'default':
    Issue bpo-26983: float() now always return an instance of exact float.
    https://hg.python.org/cpython/rev/050e5f803999

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you for your review Mark.

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Jun 6, 2016

    test_format resulted in semi-failure due to this change. The attached patch fixes the issue.

    @SilentGhost SilentGhost mannequin reopened this Jun 6, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 6, 2016

    New changeset 6216fb8afa53 by Serhiy Storchaka in branch 'default':
    Issue bpo-26983: Fixed test_format failure.
    https://hg.python.org/cpython/rev/6216fb8afa53

    @serhiy-storchaka
    Copy link
    Member Author

    My failure. Thank you for your report and patch SilentGhost.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants