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

Making a class formattable as hex/oct integer with printf-style formatting requires both __int__ and __index__ for no good reason #77183

Closed
MojoVampire mannequin opened this issue Mar 5, 2018 · 5 comments
Labels
3.7 3.8 interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs)

Comments

@MojoVampire
Copy link
Mannequin

MojoVampire mannequin commented Mar 5, 2018

BPO 33002
Nosy @mdickinson, @ericvsmith, @serhiy-storchaka, @MojoVampire, @tirkarthi
Superseder
  • bpo-36048: Deprecate implicit truncating when convert Python numbers to C integers: use index, not int
  • 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 2019-03-14.14:58:36.318>
    created_at = <Date 2018-03-05.21:07:14.038>
    labels = ['interpreter-core', '3.7', '3.8']
    title = 'Making a class formattable as hex/oct integer with printf-style formatting requires both __int__ and __index__ for no good reason'
    updated_at = <Date 2019-03-14.14:58:36.316>
    user = 'https://github.com/MojoVampire'

    bugs.python.org fields:

    activity = <Date 2019-03-14.14:58:36.316>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-03-14.14:58:36.318>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2018-03-05.21:07:14.038>
    creator = 'josh.r'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33002
    keywords = []
    message_count = 5.0
    messages = ['313285', '313286', '313288', '313311', '337928']
    nosy_count = 5.0
    nosy_names = ['mark.dickinson', 'eric.smith', 'serhiy.storchaka', 'josh.r', 'xtreak']
    pr_nums = []
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '36048'
    type = None
    url = 'https://bugs.python.org/issue33002'
    versions = ['Python 3.7', 'Python 3.8']

    @MojoVampire
    Copy link
    Mannequin Author

    MojoVampire mannequin commented Mar 5, 2018

    In Python 2, making a user-defined class support formatting using the integer-specific type codes required that __int__ be defined and nothing else (that is, '%x' % Foo() only required Foo to provide a __int__ method). In Python 3, this was changed to perform the conversion via __index__ for the %o, %x and %X format types (to match how oct and hex behave), not __int__, but the pre-check for validity in unicodeobject.c's mainformatlong function is still based on PyNumber_Check, not PyIndex_Check, and PyNumber_Check is concerned solely with __int__ and __float__, not __index__.

    This means that a class with __index__ but not __int__ can't be used with the %o/%x/%X format codes (even though hex(mytype) and oct(mytype) work just fine).

    It seems to me that either:

    1. PyNumber_Check should be a superset of PyIndex_Check (broader change, probably out of scope)

    or

    1. mainformatlong should restrict the scope of the PyNumber_Check test to only being used for the non-'o'/'x'/'X' tests (where it's needed to avoid coercing strings and the like to integer).

    Change #2 should be safe, with no major side-effects; since PyLong and subclasses always passed the existing PyNumber_Check test anyway, and PyNumber_Index already performs PyIndex_Check, the only path that needs PyNumber_Check is the one that ends in calling PyNumber_Long.

    @MojoVampire MojoVampire mannequin added 3.7 3.8 interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) labels Mar 5, 2018
    @MojoVampire
    Copy link
    Mannequin Author

    MojoVampire mannequin commented Mar 5, 2018

    Note: Obviously, defining __index__ without defining __int__ is a little strange (it's *equivalent* to int, but can't be *coerced* to int?), so yet another fix would be addressing bpo-20092 so it wouldn't be possible for a type to define __index__ without (implicitly) defining __int__.

    @MojoVampire
    Copy link
    Mannequin Author

    MojoVampire mannequin commented Mar 5, 2018

    To be clear, this is a problem with old-style (printf-style) formatting, and applies to both bytes formatting and str formatting. So a class like:

        class Foo:
            def __index__(self):
                return 1

    will fail with a TypeError should you do any of:

    '%o' % Foo()
    '%x' % Foo()
    '%X' % Foo()
    b'%o' % Foo()
    b'%x' % Foo()
    b'%X' % Foo()
    

    even though hex(Foo()) and oct(Foo()) work without issue.

    @MojoVampire MojoVampire mannequin changed the title Making a class formattable as hex/oct integer requires both __int__ and __index__ for no good reason Making a class formattable as hex/oct integer with printf-style formatting requires both __int__ and __index__ for no good reason Mar 5, 2018
    @mdickinson
    Copy link
    Member

    mdickinson commented Mar 6, 2018

    Agreed that this seems wrong.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Mar 14, 2019

    Resolved in bpo-36048.

    @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 3.8 interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants