I made a modification in typeobject.c to allow __class__ modification for classes with slots. It's basically a change in same_slots_added to count the offset of the slots and check if the names of the slots are the same (in the naive way). 

I don't check if slots are in a different order, that may be an improve.

The patch is against trunk, with some tests. It's my first submission on Python, so every feedback will be welcome :).

Just going through the list doesn't seem so naive to me.  If the slots are in a different order, then you would need to move the data around -- which borders on "maybe they ought to have written the translator themselves"

Is there anything I can do to have a resolution on this ?

Thanks.

Review five other patches. 

Post the review summaries (and tracker numbers) to the python-dev mailing list.

In that same message, ask someone with commit privileges to do the 5:1 deal, pointing at this tracker number.

You should use _PyString_Eq() for string comparison instead
of relying on the internal details of PyStringObject.

File Added: same_slots_added_2.diff

Thanks, I wasn't able to find this function.

I updated the patch for using this.
File Added: same_slots_added_2.diff

The PyObject * cast is reduntant, but otherwise the
patch looks good to me. I added a few more tests,
but I can't attach a file so I posted them here:

http://freeweb.siol.net/chollus/

File Added: same_slots_added_3.diff

The only problem that I see with this patch is that
slot names have to be listed in both classes in the
same order for __class__ assignment to work. This
could have strange consequences when a dict is used
for __slots__.  

I'm attaching a patch that fixes this, with modified
test and documentation. The change had to be done
in type_new() function, by sorting slotnames before
the creation of member descriptors. Most of the
documentation changes are whitespace cleanup.

File Added: same_slots_added_4.diff

That's great, thanks a lot for you help.

Ziga, there are a lot of changes to the doc that appear to be whitespace only.  Could you review them from the patch before checkin?  If you want to make whitespace/formatting changes, that's fine, but it's a lot easier to review if there are 2 separate checkins.

In the test, it looks like it would be much easier to unpack the classes in the loop, since you know there are two.  That would eliminate 2 of the inner loops which don't seem necessary.

  for cls1, cls2 in ((G, H), (P, Q)):

Also, when doing data driven tests, it's important to add an error message about which iteration failed.  Right now, you wouldn't know if G/H failed or P/Q failed since the line number will be the same.  If you add a message to the assertions which contains data, then it's clear from the message which one failed.

typeobject.c

I see _PyString_Eq is used.  It's possible that __slots__ contain unicode, possibly other types (I didn't check).  What happens if non-strings are in __slots__ with this patch applied?

The calculation for size could be done outside the loop with a multiply.  I don't know which would be faster.  I don't really care which is used either.  Just something I noticed.

> Ziga, there are a lot of changes to the doc that appear to be whitespace
> only.  Could you review them from the patch before checkin?

I guess you mean "remove them" ?

> I see _PyString_Eq is used.  It's possible that __slots__ contain unicode,
> possibly other types (I didn't check).

It seems __slots__ can be unicode, but only with ascii content. This behavior is a bit weird...

The ht_slots tuple can't contain unicode names;
they are converted to strings in type_new(). type_new()
also raises an error for any non string object found in
__slots__. At the end of type_new(), ht_slots tuple can
only contain valid identifiers minus __weakref__ and
__dict__.

I changed the patch to use PyObject_Compare() anyway,
because it reduces the amount of code. I also removed
whitespace changes in the docs, changed the tests as
suggested and added a test with unicode slots.

File Added: same_slots_added_5.diff

Sorry about the typo, I did mean remove, not review.  The latest patch looks good.  

I have some questions though.  What would happen if an exception is raised when comparing slots?  I think it will be ok, the exception would get overwritten with the one about incompatible types/__slots__.  

I just want to be safe and it might be good to at least test this manually.  Is the original object saved in ht_slots or is it always a tuple?  If the original object is used, what if you have two classes with the same slots, but one uses a list and the other uses a tuple for __slots__?

Hi Neal,

Yes, an exception in slots comparison would get
overwritten, but I think that this is highly
unlikely to happen. The user would have to put a
malicious string into __slots__, and that string
would have to go through list sorting in type_new()
without raising an error.

The original object is never stored in ht_slots;
type_new() always constructs a new tuple from the
sorted slot names.

I'll leave the patch open until tomorow in case you
have any other concerns, and then check it in.

Go ahead and check in.  Thanks!

Commited as revision 54412. Thanks for the patch, Thomas!