Add dunder overriding

[alendit]

Related to #11, although right now it doesn't fix that issue exactly. This patch add the overloading of many dunder methods, i.e. "add" or "matmul" for built-in types.

Implementing "str" and "repr" should be easy enough, I'll look into it, if the patch direction is approved.

[clarete]

This is incredible to say the least! Thanks for taking the time to play with the library and to send such fun contribution! I have a lil question before merging it. Do you think you could rewrite that test to use another operator instead of the @ so it doesn't cause syntax error for tests on other python versions?

Excited with all the possibilities open by this change \o/

[clarete]

Sorry for the minor nitpick but I'd get rid of this line! :)

[alendit]

Hey, Thanks for looking into it so quickly. I'll clean up the code a little in the following days (hopefully tomorrow), make the suggested changes and ping you again.

…

On Mon, Feb 18, 2019, 18:09 Lincoln de Sousa ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In forbiddenfruit/__init__.py <#24 (comment)>: > class PyObject(ctypes.Structure): pass +def make_opaque_ptr(name): + newcls = type(name, (ctypes.Structure,), {}) + return ctypes.POINTER(newcls) + + +#PyObject_p = ctypes.POINTER(PyObject) Sorry for the minor nitpick but I'd get rid of this line! :) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#24 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAE83aIEmU0AgN429SKa_Pqdy1yieOgwks5vOt4sgaJpZM4bA67c> .

[alendit]

Hi,

so couple of additions: you can now throw NotImplementedError in the overridden dunders which will make the interpreter proceed with overload resolution. It's useful for type checking the arguments. I also made the overriding revertable.

We'd have to look into supporting older python version, because they likely have a different PyTypeObject layout. The principle should be the same. With python 2.7 EOL nearing, you could consider dropping the support of the legacy versions in newer releases of your library, but it's another discussion.

[clarete]

Very cool changes! Really liked the possibility to revert the patching of the dunders!

One question though! Do you think there's any possibility to try to get the same functionality of those two methods written in C using the cffi? Not really a blocker but not having to compile the C code might make it easier to install and play with the library! If there's no good solution for that, let me know if you need any help making the tests work.

To your point about the python versions. That's indeed a good thing to think about. The test suite still tries to use python 2.5 although not even travis supports that anymore! I have no idea about which version we should actually support (besides 3.6+). I'd love to hear what you think!

[alendit]

Yeah, thought of the way to avoid having a native module. It relies on kind of undocumented behaviour of ctypes, but the library's name isn't allowedfruit...

I also added a bunch of tests and a skip decorator for the older versions.

As for legacy support: I see a real use case for this library in something one-offish, like a Jupyter notebook, where you want to have some nice syntax, but not concerned that much about the stability in many different environments. To my knowledge, nobody runs python <2.7 there, and even 2.7 becomes rarer and rarer.

[alendit]

Used your cool PyDict_SetItem trick to get the PyNotImplemented, which makes the code look cleaner.

[clarete]

This is awesome! Thanks a lot for the updates on the patch! They look great! I'm ready to merge them, let me know if you're OK with the merge or if you want to do anything else before that!

On the python compatibility topic, I guess we really don't need 2.5 & 2.6 anymore! I'd keep 2.7 for now. What's your take on which python 3 versions should be supported?

[alendit]

I'd say you can merge it. The test failure is unrelated to the changes, I think. The test tries to overwrite pop method of the dict. pop isn't defined in the __dict__ though. Instread there's a custom struct on the dict type which holds references to the built-in method implementations. Not sure why the tests succeeds from time to time, though.

Yeah, it makes sense to support 2.7 at least until the EOL. I can look into supporting it for this patch. Most likely the PyTypeObject layout is slightly different, but it's unlikely that the difference is huge.

As for Python 3, I think everything >=3.3 is a legitimate target.

[clarete]

I'd say you can merge it. The test failure is unrelated to the changes, I think. The test tries to overwrite pop method of the dict. pop isn't defined in the __dict__ though. Instread there's a custom struct on the dict type which holds references to the built-in method implementations. Not sure why the tests succeeds from time to time, though.

Thanks a lot for the debugging on this issue! Had no idea it was the case! Always good to learn more about python! :)

Yeah, it makes sense to support 2.7 at least until the EOL. I can look into supporting it for this patch. Most likely the PyTypeObject layout is slightly different, but it's unlikely that the difference is huge.

That'd be great! But I feel like it's a good time to merge this PR since as you said, your additions didn't really change the tests (only for the better!!)

As for Python 3, I think everything >=3.3 is a legitimate target.

Nice! Thanks for the comment! I'll get the travis updated! <o/