Skip to content
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

Dunder attributes added to wrapped function objects #282

Open
lazorchakp opened this issue Jan 30, 2025 · 11 comments
Open

Dunder attributes added to wrapped function objects #282

lazorchakp opened this issue Jan 30, 2025 · 11 comments

Comments

@lazorchakp
Copy link

I've come across a case similar to #93 - I have wrapped a function using a wrapt.function_wrapper, but this adds attributes (dunder methods) to the resulting function that are not present on the unwrapped function.

The original issue discusses several difficulties with implementing a fix on ObjectProxy. I'm wondering if limiting the scope of a fix to just FunctionWrapper would be more reasonable / acceptable.

A simplified demonstration of the issue:

import wrapt

def my_func(): pass

@wrapt.function_wrapper
def wrapper(wrapped, instance, args, kwargs):
    return wrapped(*args, **kwargs)

wrapped_func = wrapper(my_func)

assert not hasattr(my_func, "__add__")
assert not hasattr(wrapped_func, "__add__")  # fails

I'm mostly curious about the feasibility of defining __getattribute__ on FunctionWrapper (or possibly _FunctionWrapperBase). I understand that doing this directly on ObjectProxy would have too much of a negative performance impact, but I don't expect attribute access to occur often enough on function objects for this to be a concern for FunctionWrappers. I've tried out a few fixes, but so far haven't been able to come up with a correct implementation.

@GrahamDumpleton
Copy link
Owner

Am aware of complications that can occur for this in certain cases. Can you explain how this is causing an issue in your specific case?

The cases I am aware of tend to resolve around special methods like __call__ and __iter__. That you mention __add__ is a new one on me as to what may cause issues, so any extra details you can share would help in understanding how it can cause problems.

@lazorchakp
Copy link
Author

Sorry, I was trying to simplify as much as possible and used __add__ strictly as an example. The exact method that is causing issues for me is __iter__, as you suspected. I came across a function in pandas that accepts an unknown object and tries to determine if it is iterable, and that function is incorrectly identifying wrapped function objects as iterable.

@GrahamDumpleton
Copy link
Owner

Yep, that is the exact one I mainly know about causing problems.

The thought on that one, but haven't got around to trying it, is not to have the __iter__ wrapper directly on the class declaration but check for whether it exists on the wrapped object in the __init__ method of the ObjectProxy. I'd like to avoid having to do this sort of check for too many different special methods though in the constructor. Also need to consider it for __call__ and async iterator methods.

@lazorchakp
Copy link
Author

That's an interesting idea. I won't have a chance to try it out until next week, but it sounds promising. I'll let you know how it goes.

@lazorchakp
Copy link
Author

The following update to the pure-python implementation seems to be working for my case, and the without-extensions tests are passing locally.

class ObjectProxy(with_metaclass(_ObjectProxyMetaType)):
    # [...]
    def __init__(self, wrapped):
        # [...]
        try:
            object.__setattr__(self, '__iter__', wrapped.__iter__)
        except AttributeError:
            pass

Not sure if this is the most correct or efficient way to do things though. Would you consider adding something like this?

@GrahamDumpleton
Copy link
Owner

Something like that is indeed what I was thinking of. Although I was stupidly thinking that would need to have a hasattr() check rather than just use setattr() and ignore exception if it fails.

@lazorchakp
Copy link
Author

Thanks for the guidance so far. Unfortunately I haven't yet been able to get something similar working in the C extension. Do you have any hints or ideas for what this might look like in extension code?

If it helps, I've tried removing WraptObjectProxy_iter entirely and setting WraptObjectProxy_Type's tp_iter to 0, then in WraptObjectProxy_raw_init following the same strategy as __module__ and __doc__, but with __iter__. This almost works, but it breaks iteration for wrapped objects that are actually iterable (these wrappers have an __iter__, but [x for x in wrapper] still produces a TypeError).

@GrahamDumpleton
Copy link
Owner

Can you show the actual code you used along with full description from the error.

Want to double check no reference counting issues. Also, some of the dunder methods are a bit special and need to be on the class type when it is implemented in C and having them as attribute is a problem. Am not aware of iter being one of those though.

@lazorchakp
Copy link
Author

Here's the diff: develop...lazorchakp:wrapt:conditionally-iterable-object-proxies

The full error from WRAPT_INSTALL_EXTENSIONS=true pip install . && pytest is:

================================ FAILURES =================================
___________________ TestIterObjectProxy.test_iteration ____________________

self = <test_object_proxy.TestIterObjectProxy testMethod=test_iteration>

    def test_iteration(self):
        items = [1, 2]

        wrapper = wrapt.ObjectProxy(items)

        self.assertTrue(hasattr(wrapper, "__iter__"))
>       result = [x for x in wrapper]
E       TypeError: 'ObjectProxy' object is not iterable

tests/test_object_proxy.py:787: TypeError

Tests pass with WRAPT_INSTALL_EXTENSIONS=false. I've been testing (arbitrarily) with python 3.11.7, and my platform.platform() is macOS-15.3-arm64-arm-64bit.

@GrahamDumpleton
Copy link
Owner

What happens if just do:

iter(wrapper)

Does that even fail?

@lazorchakp
Copy link
Author

This fails with the same error:

        self.assertTrue(hasattr(wrapper, "__iter__"))
>       _ = iter(wrapper)
E       TypeError: 'ObjectProxy' object is not iterable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants