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

complex() gives wrong error when the second argument has an invalid type #72390

Closed
Manishearth mannequin opened this issue Sep 19, 2016 · 15 comments
Closed

complex() gives wrong error when the second argument has an invalid type #72390

Manishearth mannequin opened this issue Sep 19, 2016 · 15 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@Manishearth
Copy link
Mannequin

Manishearth mannequin commented Sep 19, 2016

BPO 28203
Nosy @mdickinson, @berkerpeksag, @serhiy-storchaka, @soummyaah
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • Issue28203.patch: diff complexobject.c
  • Issue28203#3.patch
  • Issue28203#4.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 2016-09-24.14:32:59.845>
    created_at = <Date 2016-09-19.07:41:45.974>
    labels = ['3.7', 'type-bug', 'library']
    title = 'complex() gives wrong error when the second argument has an invalid type'
    updated_at = <Date 2017-03-31.16:36:39.326>
    user = 'https://bugs.python.org/manishearth'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:39.326>
    actor = 'dstufft'
    assignee = 'mark.dickinson'
    closed = True
    closed_date = <Date 2016-09-24.14:32:59.845>
    closer = 'mark.dickinson'
    components = ['Library (Lib)']
    creation = <Date 2016-09-19.07:41:45.974>
    creator = 'manishearth'
    dependencies = []
    files = ['44745', '44754', '44755']
    hgrepos = ['356']
    issue_num = 28203
    keywords = ['patch']
    message_count = 15.0
    messages = ['276952', '276954', '276958', '276970', '276972', '277003', '277013', '277022', '277023', '277025', '277026', '277108', '277318', '277319', '277353']
    nosy_count = 6.0
    nosy_names = ['mark.dickinson', 'python-dev', 'berker.peksag', 'serhiy.storchaka', 'manishearth', 'soummyaah']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28203'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @Manishearth
    Copy link
    Mannequin Author

    Manishearth mannequin commented Sep 19, 2016

    When the second argument of complex() is not a number/string, the type error reports the error but prints the type of the first argument:

        > complex({1:2},1j)
        Traceback (most recent call last):
         File "<stdin>", line 1, in <module>
        TypeError: complex() argument must be a string or a number, not 'dict'
        >complex(1j,{1:2})
        Traceback (most recent call last):
         File "<stdin>", line 1, in <module>
        TypeError: complex() argument must be a string or a number, not 'complex'
        >>> complex(1, {1:2})
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
        TypeError: complex() argument must be a string or a number, not 'int'

    @Manishearth Manishearth mannequin added type-feature A feature request or enhancement 3.7 (EOL) end of life stdlib Python modules in the Lib dir labels Sep 19, 2016
    @soummyaah
    Copy link
    Mannequin

    soummyaah mannequin commented Sep 19, 2016

    Contains changes made to Objects/complexobject.c
    Changed the error message returned when second argument of complex() is not number/string
    Originally:
    >complex(1j,{1:2})
        Traceback (most recent call last):
         File "<stdin>", line 1, in <module>
        TypeError: complex() argument must be a string or a number, not 'complex'
    
    After patch:
    >complex(1j,{1:2})
        Traceback (most recent call last):
         File "<stdin>", line 1, in <module>
        TypeError: complex() argument must be a string or a number, not 'dict'

    @SilentGhost SilentGhost mannequin added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Sep 19, 2016
    @berkerpeksag
    Copy link
    Member

    Please upload your patch from a Mercurial clone:

    Currently, if you pass a string as a second argument, you get:

    >>> complex(1, "1")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: complex() second arg can't be a string

    So the exception message should probably be changed to include "second arg" or "second argument":

    >>> complex(1j, {1: 2})
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: complex() second arg must be a number, not 'dict'

    Also, there is already a check for second argument in line 952 so the "must be a string" part is probably not needed. We probably need to check whether these two cases can be combined.

    You also need to add some tests to Lib/test/test_complex.py.

    @mdickinson
    Copy link
    Member

    @Manishearth: Nice catch! Thanks for the report.

    @soummyaah: Thanks for the patch. Please could you sign a contributor agreement[1], so that we can commit the patch? As Berker says, tests would be good to have, too.

    [1] https://www.python.org/psf/contrib/contrib-form/

    @mdickinson
    Copy link
    Member

    This should be fixed in Python 3.5, too.

    @soummyaah
    Copy link
    Mannequin

    soummyaah mannequin commented Sep 20, 2016

    I've signed the contributor agreement form. I think it said that it'll take a few days to process?
    I've made the changes requested and am currently working on the tests. Will submit a new patch file containing all required changes soon.

    @mdickinson
    Copy link
    Member

    Thank you! I look forward to the new patch.

    @soummyaah
    Copy link
    Mannequin

    soummyaah mannequin commented Sep 20, 2016

    Changed error message to:

    >>> complex({1:2},1)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: complex() first arg must be a string or a number, not 'dict'
    
    >>> complex(1j, {1: 2})
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: complex() second arg must be a number, not 'dict'

    Added tests to check the error raised.

    @soummyaah
    Copy link
    Mannequin

    soummyaah mannequin commented Sep 20, 2016

    Apologies. This is the correct file.

    @soummyaah
    Copy link
    Mannequin

    soummyaah mannequin commented Sep 20, 2016

    Squashed the commits for better readability.
    Also, change required in Python 3.4 as well

    @berkerpeksag
    Copy link
    Member

    Thanks for the patch. We can't fix this in 3.4 because it's in security-fix-only mode: https://docs.python.org/devguide/index.html#status-of-python-branches

    @mdickinson
    Copy link
    Member

    Thanks. I'll apply this shortly (but probably not before the weekend).

    @mdickinson mdickinson self-assigned this Sep 21, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 24, 2016

    New changeset 92f4ce2d5ebb by Mark Dickinson in branch '3.5':
    Issue bpo-28203: Fix incorrect type in error message from complex(1.0, {2:3}). Patch by Soumya Sharma.
    https://hg.python.org/cpython/rev/92f4ce2d5ebb

    New changeset a2d93e6bcbcf by Mark Dickinson in branch '3.6':
    Issue bpo-28203: Merge from 3.5
    https://hg.python.org/cpython/rev/a2d93e6bcbcf

    New changeset 9790bc211107 by Mark Dickinson in branch 'default':
    Issue bpo-28203: Merge from 3.6
    https://hg.python.org/cpython/rev/9790bc211107

    @mdickinson
    Copy link
    Member

    Fixed; thanks. I made a couple of changes:

    • Use "argument" rather than "arg", to be consistent with the original code (but admittedly not consistent with the rest of the module, where there doesn't seem to be any consistent choice between "arg" and "argument").
    • Reformat C and test code to avoid long lines.
    • Slight rearrangement of the C code so that all of the "i" handling is in one place.

    @soummyaah
    Copy link
    Mannequin

    soummyaah mannequin commented Sep 25, 2016

    Thanks for the merge!

    @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
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants