[proxy] web.archive.org← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public
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

bpo-20092: Make __int__ defaults to __index__ #13106

Closed

Conversation

Copy link
Contributor

remilapeyre commented May 6, 2019

serhiy-storchaka changed the title bpo-33039: Make __int__ defaults to __index__ bpo-20092: Make __int__ defaults to __index__ May 6, 2019
Objects/typeobject.c Outdated Show resolved Hide resolved
Copy link
Contributor

jdemeyer left a comment

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.

Copy link
Contributor Author

@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.

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

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.

Copy link
Contributor Author

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'

Copy link
Contributor

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

Copy link
Contributor Author

@jdemeyer, thanks, I pushed my last commit

Copy link
Contributor Author

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

remilapeyre closed this Jun 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants