-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix __class__ #619
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
Fix __class__ #619
Conversation
Codecov Report
@@ Coverage Diff @@
## master #619 +/- ##
==========================================
+ Coverage 41% 41.12% +0.12%
==========================================
Files 76 76
Lines 17184 17207 +23
Branches 4514 4508 -6
==========================================
+ Hits 7046 7077 +31
- Misses 8140 8177 +37
+ Partials 1998 1953 -45
Continue to review full report at Codecov.
|
Are member descriptor and data descriptor actually different things or were member descriptors just missing the set (and hence not working correctly in this case)? |
They are different, member descriptors shouldn't have __set__ as the presence of it affects the lookup. In CPython: >>> type(object.__dict__['__class__'])
<class 'getset_descriptor'>
>>> type(type.__dict__['__mro__'])
<class 'member_descriptor'> I'm been thinking of a nicer way to do this in rust, instead of having different new_xxx_descriptor methods, instead a builder, which would statically work out the python type needed. vm.new_property()
.add_getter(...)
.add_setter(...)
.build() |
Great. The builder sounds nice, probably reasonable to add that later and merge this as is. |
Fixes #583
Moves the __class__ property to object, so lookup of it works properly, this matches CPython. Also to make lookup work the property has to have a __set__ method, at this point it always raises an exception.