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

Why is this not part of wrapt? #22

Closed
traverseda opened this issue Sep 5, 2017 · 5 comments
Closed

Why is this not part of wrapt? #22

traverseda opened this issue Sep 5, 2017 · 5 comments

Comments

@traverseda
Copy link

It seems like this is the kind of thing that would be better as an upstream contribution to wrapt, and not as it's own module.

@ionelmc
Copy link
Owner

ionelmc commented Sep 5, 2017

Didn't occur to me that they would have the same scope (wrapt is for decorators, LOP is simply a call-on-first-use proxy) or that @GrahamDumpleton would like to maintain this. It's a big lump of code (can't really have same C code doing both kinds of wrappings without loosing perf) + a big lump of tests after all ...

Do you want to make the contribution to wrapt or why do you ask this question?

@traverseda
Copy link
Author

traverseda commented Sep 6, 2017

I don't have time to do that right now, and was mostly just asking out of curiosity. I can't help but feel it would be better if you were both maintaining the same package, two maintainers is better than one after all. Reduces bus-factor.

It doesn't look like wrapt is for decorators to me? Their object-proxy is pretty useful, and I don't really understand why this object proxy isn't an optional-extension to that object proxy. Or rather, why that object proxy isn't an optional extension to this one, that loads on startup. Lazy-object-proxies seem like a superset of object-proxies. Being able to use isinstance on both would be pleasant.

Not very important, and probably not worth the effort, but figured I'd ask you to take a look.

@GrahamDumpleton
Copy link
Contributor

The wrapt package was originally intended for monkey patching, so the transparent object proxy was the main purpose of the library. It just happens that the object proxy is what you need to implement decorators properly. If the package was only advertised as being for monkey patching, no one would have looked at it, so from marketing perspective has been better to push it as a decorator package.

Is the only think this package does differently is allowing for lazy creation of the wrapped object?

Doing that in C code doesn't need to add any overhead. Avoiding overhead in Python will just need a bit of magic which I can half see in my head how to do, so just need to try it.

I don't understand the comments about isinstance().

Did we already have an issue in wrapt GitHub for adding lazy construction of wrapped object? I remember us discussing this before but I can't find it. Maybe it was in email.

@traverseda
Copy link
Author

My comments on isinstance are just that it would be nice if there was a generic object-proxy object that class that different object-proxies inherited from. So isinstance(myObj, Proxy) would equal True, whether it's a lazy proxy or a standard proxy. Or, in an ideal world, some weird stuff like network-transparent proxies, like what pyro4 creates.

@GrahamDumpleton
Copy link
Contributor

FWIW, I would probably implement it so that as long as you wrap the factory function in a special type so it can be unique identified, that I overload the passed in wrapped argument.

There is a similar construct used in the decorator when supplying an adapter definition to change the type specification.

class AdapterFactory(object):
    def __call__(self, wrapped):
        raise NotImplementedError()

class DelegatedAdapterFactory(AdapterFactory):
    def __init__(self, factory):
        super(DelegatedAdapterFactory, self).__init__()
        self.factory = factory
    def __call__(self, wrapped):
        return self.factory(wrapped)

adapter_factory = DelegatedAdapterFactory

Which is used as:

    def test_adapter_factory(self):
        def factory(wrapped):
            argspec = inspect.getargspec(wrapped)
            argspec.args.insert(0, 'arg0')
            return argspec

        @wrapt.decorator(adapter=wrapt.adapter_factory(factory))
        def _wrapper_1(wrapped, instance, args, kwargs):
            return wrapped(*args, **kwargs)

The actual code then has a check:

            if adapter:
                if isinstance(adapter, AdapterFactory):
                    adapter = adapter(wrapped)

In the case we are talking about would use it as:

import wrapt

def expensive_func():
    from time import sleep
    print('starting calculation')
    # just as example for a very slow computation
    sleep(2)
    print('finished calculation')
    # return the result of the calculation
    return 10

obj = wrapt.Proxy(wrapt.wrapped_factory(expensive_func))
# function is called only when object is actually used
print(obj)  # now expensive_func is called

print(obj)  # the result without calling the expensive_func

IOW, based on see special factory type, change what happens internally such that flagged that need to do lazy setting of wrapped object first time it is required.

So you don't need a special type of the lazy case, would be part of the one proxy class implementation.

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

3 participants