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

Conversion from fractions.Fraction to bool #83455

Closed
francois-durand mannequin opened this issue Jan 9, 2020 · 12 comments
Closed

Conversion from fractions.Fraction to bool #83455

francois-durand mannequin opened this issue Jan 9, 2020 · 12 comments
Labels
3.9 only security fixes easy type-bug An unexpected behavior, bug, or error

Comments

@francois-durand
Copy link
Mannequin

francois-durand mannequin commented Jan 9, 2020

BPO 39274
Nosy @rhettinger, @mdickinson, @vstinner, @seberg, @miss-islington, @idomic, @francois-durand
PRs
  • bpo-39274: Ensure Fraction.__bool__ returns a bool #18017
  • [3.8] bpo-39274: Ensure Fraction.__bool__() returns a bool (GH-18017) #18376
  • [3.7] bpo-39274: Ensure Fraction.__bool__() returns a bool (GH-18017) #18377
  • 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 = None
    closed_at = <Date 2020-02-06.14:59:07.888>
    created_at = <Date 2020-01-09.12:43:51.476>
    labels = ['easy', 'type-bug', '3.9']
    title = 'Conversion from fractions.Fraction to bool'
    updated_at = <Date 2020-02-06.15:14:42.456>
    user = 'https://github.com/francois-durand'

    bugs.python.org fields:

    activity = <Date 2020-02-06.15:14:42.456>
    actor = 'miss-islington'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-02-06.14:59:07.888>
    closer = 'vstinner'
    components = []
    creation = <Date 2020-01-09.12:43:51.476>
    creator = 'francois-durand'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39274
    keywords = ['patch', 'newcomer friendly']
    message_count = 12.0
    messages = ['359673', '359675', '359677', '359679', '359681', '360077', '361237', '361238', '361488', '361489', '361491', '361492']
    nosy_count = 8.0
    nosy_names = ['rhettinger', 'mark.dickinson', 'vstinner', 'SilentGhost', 'seberg', 'miss-islington', 'Ido Michael', 'francois-durand']
    pr_nums = ['18017', '18376', '18377']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue39274'
    versions = ['Python 3.9']

    @francois-durand
    Copy link
    Mannequin Author

    francois-durand mannequin commented Jan 9, 2020

    As of now, fractions.Fraction.bool is implemented as: return a._numerator != 0. However, this does not necessary return a bool (which would be desired). In particular, when the numerator is a numpy integer, this returns a numpy bool instead. Another solution would be to implement fractions.Fraction.bool as: return bool(numerator). What do you think?

    This message follows a thread here: numpy/numpy#15277 .

    @francois-durand francois-durand mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes type-bug An unexpected behavior, bug, or error labels Jan 9, 2020
    @mdickinson
    Copy link
    Member

    Agreed that bool should return an actual bool.
    return bool(a._numerator) does seem like the obvious way to do this.

    @mdickinson
    Copy link
    Member

    For completeness and to save people going to the NumPy tracker, here's an example of the problem:

    Python 3.8.1 (default, Jan  5 2020, 21:32:35) 
    [Clang 10.0.1 (clang-1001.0.46.4)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import fractions, numpy
    >>> f = fractions.Fraction(numpy.int64(1), 2)
    >>> not f
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: __bool__ should return bool, returned numpy.bool_

    @vstinner
    Copy link
    Member

    vstinner commented Jan 9, 2020

    return bool(a._numerator) does seem like the obvious way to do this.

    Yes, it's a good fix. It's used on other places.

    --

    numbers.Complex.__bool__() also uses "self != 0". Is it an issue?

    @seberg
    Copy link
    Mannequin

    seberg mannequin commented Jan 9, 2020

    Thanks for the quick responses.

    @victor Stinner, I suppose you could change numbers.Complex.__bool__() by adding the no-op bool to make it: bool(self != 0).

    But I am not sure I feel it is necessary. NumPy is a bit a strange in that it uses its own boolean scalar, and it is easy to override the slot for such objects.

    @vstinner
    Copy link
    Member

    Does someone want to propose a PR to modify fractions.Fraction.__bool__() to use "return bool(a._numerator)"?

    @vstinner vstinner added easy and removed 3.7 (EOL) end of life 3.8 only security fixes labels Jan 15, 2020
    @idomic
    Copy link
    Mannequin

    idomic mannequin commented Feb 2, 2020

    Hi all,

    I think this issue can be closed right?
    Saw a successful PR.

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Feb 2, 2020

    "Successful" PR would be merged into master. This issue is still in "patch review" stage.

    @vstinner
    Copy link
    Member

    vstinner commented Feb 6, 2020

    New changeset 427c84f by Sebastian Berg in branch 'master':
    bpo-39274: Ensure Fraction.__bool__() returns a bool (GH-18017)
    427c84f

    @vstinner
    Copy link
    Member

    vstinner commented Feb 6, 2020

    Thanks Sebastian Berg for the fix, and thanks François Durand for the bug report! This issue is fixed in the master branch, and backports to 3.7 and 3.8 will land soon automatically (once the CI tests pass).

    @miss-islington
    Copy link
    Contributor

    New changeset 0d03a10 by Miss Islington (bot) in branch '3.8':
    bpo-39274: Ensure Fraction.__bool__() returns a bool (GH-18017)
    0d03a10

    @miss-islington
    Copy link
    Contributor

    New changeset 705d271 by Miss Islington (bot) in branch '3.7':
    bpo-39274: Ensure Fraction.__bool__() returns a bool (GH-18017)
    705d271

    @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.9 only security fixes easy type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants