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

PyComplex_AsCComplex should allow __complex__ to return float. #60494

Closed
mdickinson opened this issue Oct 20, 2012 · 10 comments
Closed

PyComplex_AsCComplex should allow __complex__ to return float. #60494

mdickinson opened this issue Oct 20, 2012 · 10 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@mdickinson
Copy link
Member

BPO 16290
Nosy @jcea, @mdickinson, @serhiy-storchaka, @antocuni
Files
  • issue16290.patch
  • issue16290v2.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 2012-11-14.17:09:03.015>
    created_at = <Date 2012-10-20.09:39:39.888>
    labels = ['interpreter-core', 'type-feature']
    title = 'PyComplex_AsCComplex should allow __complex__ to return float.'
    updated_at = <Date 2012-11-14.17:09:03.015>
    user = 'https://github.com/mdickinson'

    bugs.python.org fields:

    activity = <Date 2012-11-14.17:09:03.015>
    actor = 'mark.dickinson'
    assignee = 'mark.dickinson'
    closed = True
    closed_date = <Date 2012-11-14.17:09:03.015>
    closer = 'mark.dickinson'
    components = ['Interpreter Core']
    creation = <Date 2012-10-20.09:39:39.888>
    creator = 'mark.dickinson'
    dependencies = []
    files = ['27631', '27764']
    hgrepos = []
    issue_num = 16290
    keywords = ['patch']
    message_count = 10.0
    messages = ['173379', '173380', '173384', '173386', '173389', '173390', '174030', '174095', '174105', '175576']
    nosy_count = 5.0
    nosy_names = ['jcea', 'mark.dickinson', 'python-dev', 'serhiy.storchaka', 'antocuni']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'needs patch'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16290'
    versions = ['Python 3.4']

    @mdickinson
    Copy link
    Member Author

    See thread starting at http://mail.python.org/pipermail/python-dev/2012-October/122241.html

    The cmath module functions don't accept custom types that define __complex__, but whose __complex__ method returns a float rather than a complex number:

    Python 3.4.0a0 (default:53a7c2226a2b+, Oct 19 2012, 12:16:36) 
    [GCC 4.2.1 (Apple Inc. build 5664)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> class A:
    ...     def __complex__(self): return 42.0
    ... 
    >>> import cmath
    >>> cmath.sqrt(A())
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: __complex__ should return a complex object

    In contrast, the complex constructor is happy to accept such objects.

    The root cause is that PyArg_ParseTuple's 'D' format calls PyComplex_AsCComplex (in Objects/complexobject.c), which refuses to accept a __complex__ result of type float.

    @mdickinson mdickinson self-assigned this Oct 20, 2012
    @mdickinson mdickinson added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Oct 20, 2012
    @mdickinson
    Copy link
    Member Author

    Here's a patch

    @serhiy-storchaka
    Copy link
    Member

    Patch LGTM.

    Is there a new feature, not a bugfix? If yes, then I expect some documentation changes.

    @mdickinson
    Copy link
    Member Author

    Is there a new feature, not a bugfix?

    There's an ongoing argument about that on the python-dev thread. :-) But yes, I think this needs a doc update---even if it's considered a bugfix, the current docs could be clarified.

    http://docs.python.org/dev/reference/datamodel.html?highlight=__complex__#object.\_\_complex__

    says

    "Should return a value of the appropriate type." My interpretation of that is that __int__ should return an int, __float__ a float, __complex__ a complex number; with this change, that interpretation no longer works.

    I'll update the patch (and add a Misc/NEWS entry, too).

    Thanks!

    @serhiy-storchaka
    Copy link
    Member

    My interpretation of that is that __int__ should return an int, __float__ a
    float, __complex__ a complex number;

    It's a reasonable interpretation. Changing it can be confusing.

    On the other hand, int and float look like specialized subclasses of complex
    (they even have .real, .imag and .conjugate()). The ducktype principle
    requires that we can use ints or floats everywhere where complexes needed. This
    means that you should add a branch for integers too.

    @mdickinson
    Copy link
    Member Author

    This means that you should add a branch for integers too.

    I'd rather not go that far, at least in this issue: we'd also end up changing __float__ to allow it to return an int, in that case. If we're considering that, the discussion should go back to python-dev / python-ideas.

    One difference is that int -> float is a lossy conversion that has to care about details like rounding and overflow. float -> complex in contrast is much simpler.

    @mdickinson
    Copy link
    Member Author

    I've been convinced by the python-dev discussion that Antonio was right: it's complex_new that's in error rather than the cmath functions. Even so, it would be unsafe to change the behaviour in the maintenance releases.

    Attaching a new patch that disallows 'float'-type returns from the __complex__ method.

    @jcea
    Copy link
    Member

    jcea commented Oct 29, 2012

    Mark, for the sake of keeping a reference in this bug entry, could you possibly post the URL of the archived python-dev thread?

    Thanks.

    @mdickinson
    Copy link
    Member Author

    Jesús: see the first message.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 14, 2012

    New changeset 399e59ad0a70 by Mark Dickinson in branch 'default':
    Issue bpo-16290: __complex__ must now always return an instance of complex.
    http://hg.python.org/cpython/rev/399e59ad0a70

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants