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

Jump to bottom

Make the implementation consistency of operator.countOf/indexOf #88724

Closed
Rupt mannequin opened this issue Jul 3, 2021 · 10 comments
Closed

Make the implementation consistency of operator.countOf/indexOf #88724

Rupt mannequin opened this issue Jul 3, 2021 · 10 comments
Assignees
@corona10
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@Rupt
Copy link
Mannequin

Rupt mannequin commented Jul 3, 2021

BPO 44558
Nosy @rhettinger, @vstinner, @serhiy-storchaka, @corona10, @miss-islington, @Rupt
PRs
  • bpo-44558: Match countOf is/== treatment to c #27007
  • bpo-44558: Make the implementation consistency of operator.indexOf #27012
  • [3.10] bpo-44558: Make the implementation consistency of operator.indexOf (GH-27012) #27029
  • [3.9] bpo-44558: Make the implementation consistency of operator.indexOf (GH-27012) #27030
  • [3.10] bpo-44558: Match countOf is/== treatment to c (GH-27007) #27053
  • [3.9] bpo-44558: Match countOf is/== treatment to c (GH-27007) #27053  #27054
  • [3.9] bpo-44558: Match countOf is/== treatment to c (GH-27007). #27055
    		•

    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/corona10'
closed_at = <Date 2021-07-07.14:56:14.153>
created_at = <Date 2021-07-03.17:25:14.747>
labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
title = 'Make the implementation consistency of operator.countOf/indexOf'
updated_at = <Date 2021-07-07.14:56:14.153>
user = 'https://github.com/Rupt'

    bugs.python.org fields:

    activity = <Date 2021-07-07.14:56:14.153>
actor = 'corona10'
assignee = 'corona10'
closed = True
closed_date = <Date 2021-07-07.14:56:14.153>
closer = 'corona10'
components = ['Library (Lib)']
creation = <Date 2021-07-03.17:25:14.747>
creator = 'rtombs'
dependencies = []
files = []
hgrepos = []
issue_num = 44558
keywords = ['patch']
message_count = 10.0
messages = ['396917', '396974', '396981', '396982', '397034', '397035', '397081', '397086', '397095', '397096']
nosy_count = 7.0
nosy_names = ['rhettinger', 'vstinner', 'docs@python', 'serhiy.storchaka', 'corona10', 'miss-islington', 'rtombs']
pr_nums = ['27007', '27012', '27029', '27030', '27053', '27054', '27055']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue44558'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
    @Rupt
    Copy link
    Mannequin Author

    Rupt mannequin commented Jul 3, 2021

    operator.countOf behaves differently in its docstring and its c and python implementations when is and == differ.

    help(countOf) ->

        countOf(a, b, /)
        Return the number of times b occurs in a.

    This could be read to say that it returns equal to sum(x is b for x in a).

    Its python implementation returns sum(x == b for x in a).

    Since its c implementation uses PyObject_RichCompareBool, it returns sum(x is b or x == b for x in a).

    Results of these implementations can differ when x is b does not imply x == b;
    that could be from an eq method, but the the float NaN is a real example since NaN != NaN.

    The issue is demonstrated with a possible fix here https://godbolt.org/z/cPT7TToG7

    Since the c version has been in the wild for decades, I suggest that it should be taken to define the function;
    the python should be updated to match it, and the docstring could say
    "Return the number of items in a which are, or which equal, b."

    I will open a pull request with these changes shortly.

    @Rupt Rupt mannequin added 3.7 (EOL) end of life 3.8 only security fixes labels Jul 3, 2021
    @Rupt Rupt mannequin assigned docspython Jul 3, 2021
    @Rupt Rupt mannequin added docs Documentation in the Doc dir 3.10 only security fixes 3.11 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error 3.7 (EOL) end of life 3.8 only security fixes labels Jul 3, 2021
    @Rupt Rupt mannequin assigned docspython Jul 3, 2021
    @Rupt Rupt mannequin added docs Documentation in the Doc dir 3.10 only security fixes 3.11 only security fixes labels Jul 3, 2021
    @corona10 corona10 removed 3.7 (EOL) end of life 3.8 only security fixes labels Jul 4, 2021
    @corona10
    Copy link
    Member

    corona10 commented Jul 5, 2021

    New changeset 0930240 by Dong-hee Na in branch 'main':
    bpo-44558: Make the implementation consistency of operator.indexOf (GH-27012)
    0930240

    @miss-islington
    Copy link
    Contributor

    New changeset 1f8486f by Miss Islington (bot) in branch '3.10':
    bpo-44558: Make the implementation consistency of operator.indexOf (GH-27012)
    1f8486f

    @miss-islington
    Copy link
    Contributor

    New changeset 9f47d87 by Miss Islington (bot) in branch '3.9':
    bpo-44558: Make the implementation consistency of operator.indexOf (GH-27012)
    9f47d87

    @vstinner
    Copy link
    Member

    vstinner commented Jul 6, 2021

    This issue can be closed, no?

    @corona10
    Copy link
    Member

    corona10 commented Jul 6, 2021

    @vstinner

    #27007 is not yet merged due to CLA issue

    @corona10
    Copy link
    Member

    corona10 commented Jul 7, 2021

    New changeset 6bd3ecf by Rupert Tombs in branch 'main':
    bpo-44558: Match countOf is/== treatment to c (GH-27007)
    6bd3ecf

    @corona10 corona10 removed the docs Documentation in the Doc dir label Jul 7, 2021
    @corona10 corona10 changed the title operator.countOf is / == inconsistency Make the implementation consistency of operator.countOf/indexOf Jul 7, 2021
    @corona10 corona10 assigned corona10 and unassigned docspython Jul 7, 2021
    @miss-islington
    Copy link
    Contributor

    New changeset 9f431dd by Miss Islington (bot) in branch '3.10':
    bpo-44558: Match countOf is/== treatment to c (GH-27007)
    9f431dd

    @corona10
    Copy link
    Member

    corona10 commented Jul 7, 2021

    New changeset 9761abf by Dong-hee Na in branch '3.9':
    [3.9] bpo-44558: Match countOf is/== treatment to c (GH-27007). (GH-27055)
    9761abf

    @corona10
    Copy link
    Member

    corona10 commented Jul 7, 2021

    Thank you Rupert for reporting and contributing!!

    @corona10 corona10 closed this as completed Jul 7, 2021
    @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
    Assignees

    @corona10 corona10

    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @corona10 @miss-islington