bpo-20092: Make int defaults to index by remilapeyre · Pull Request #13106 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits5 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

remilapeyre

Rémi Lapeyre added 2 commits

May 6, 2019 15:56

@serhiy-storchaka serhiy-storchaka changed the titlebpo-33039: Make __int__ defaults to __index__ bpo-20092: Make __int__ defaults to __index__

May 6, 2019

jdemeyer

jdemeyer

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of handling this on the level of the Python __dict__, wouldn't it be better to handle this in nb_int? That is, set nb_int to nb_index. That way, it's also guaranteed to work for extension types defining only nb_index.

@remilapeyre

@jdemeyer, I was not absolutly clear with how the methods that have a slot interact with __dict__, I tried to read the code source and to look were __dict__ or the slot were preferred over each other and I'm still not sure about the details. I couldn't find where __getattr__ would look in the slots.

I will push a commit to update the slots instead of __dict__ tonight.

@jdemeyer

I was not absolutly clear with how the methods that have a slot interact with __dict__

It's complicated, I would have to look up the details myself.

But basically, the slots are used to put entries in the __dict__ (instances of wrapper_descriptor) and the presence of a special method (but not a wrapper_descriptor) in the __dict__ adds an entry (for example slot_nb_index) in the slot. It's especially this two-way interaction that makes things more complicated.

@remilapeyre

Thanks @jdemeyer, I think the last change should be good to make nb_int defaults to nb_index

@jdemeyer

What I meant is that should only do

type->tp_as_number->nb_int = type->tp_as_number->nb_index;

and then the wrapper descriptor __int__ should be added automatically.

@remilapeyre

This doesn't seem to work, with this:


    /* If __index__ is defined but not __int__, make it default to __index__.
       Don't touch __float__ and __complex__ as there could be some loss of
       precision.
    */
    if (type->tp_as_number != NULL && type->tp_as_number->nb_int == NULL) {
        type->tp_as_number->nb_int = type->tp_as_number->nb_index;
    }

I get:

✗ ./python.exe -m test.test_index
...E....................................................
======================================================================
ERROR: test_int_defaults_to_index (__main__.BaseTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/remi/src/cpython/Lib/test/test_index.py", line 97, in test_int_defaults_to_index
    self.assertEqual(int(Test()), 4)
TypeError: int() argument must be a string, a bytes-like object or a number, not 'Test'

@jdemeyer

Please post your failing branch, otherwise it's hard to guess what the problem is.

@remilapeyre

@remilapeyre

#13108 has been merged so the issue is now resolved.