Support for type hints with adapters

[varunagrawal]

If we have a function definition with type hints, e.g.

import A

def test_function(x: A.Dog, b: A.Cat):
    ...

the exec_ call in decorator will fail since A is not in the globals dictionary. This PR updates ns to be the globals from the calling module so that the imported packages are available and the exec works as expected.

This may seem a bit hacky, but it's the only solution I can find. If you have any suggestions, I am all ears.

[GrahamDumpleton]

Can you provide a more complete example that I can run to see the problem and error?

[varunagrawal]

Sure thing. The easiest is a function that returns a matplotlib figure. I'm adding my code which uses the adapter_factory for completeness.

import matplotlib

def argspec_factory(wrapped):
    """Update the method signature of `wrapped`."""
    argspec = inspect.getfullargspec(wrapped)

    # Hack to add the "cool_option" arg to the function signature
    args_dict = argspec._asdict()
    args_dict["args"].append("cool_option")
    if args_dict["defaults"]:
        args_dict["defaults"] = tuple([*args_dict["defaults"], False])

    argspec = namedtuple('FullArgSpec',
                         args_dict.keys())._make(args_dict.values())

    new_argspec = inspect.FullArgSpec(argspec.args, argspec.varargs,
                                      argspec.varkw, argspec.defaults,
                                      argspec.kwonlyargs,
                                      argspec.kwonlydefaults,
                                      argspec.annotations)
    return new_argspec

def add_option(func):
    @wrapt.decorator(adapter=wrapt.adapter_factory(argspec_factory))
    def _add_option(func, instance, args, kwargs):

        def _arguments(*args, cool_option=False, **kwargs):
            """Extract `cool_option`"""
            return cool_option, args, kwargs

        option, args, kwargs = _arguments(*args, **kwargs)

        # Do something with option

        # Call the function.
        ret = func(*args, **kwargs)
        return ret

    return _add_option(func)

@add_option
def plot_figure(fignum: int) -> matplotlib.figure.Figure:  # Get the error `NameError: name 'matplotlib' is not defined`
    """We should be able to call `plot_figure(n, cool_option=some_cool_option)` thanks to the decorator."""
    fig = matplotlib.figure(fignum)
    return fig

[varunagrawal]

Also @GrahamDumpleton you might want to specify a compatible version of pip for python 3.5 and below since the CI is failing on pip using f-strings.

[coveralls]

Coverage increased (+2.8%) to 94.106% when pulling 3e9fb597c77526a07ccf35d2cd2dfa5a3753a327 on varunagrawal:fix/adapter-type-hints into 2f5836fa7dc19cd8cff9926b3d688b886445bbef on GrahamDumpleton:develop.

[varunagrawal]

Made a bunch of updates to get CI to pass.

[varunagrawal]

@GrahamDumpleton I figured out a more elegant way to fix this. We can just remove all the type hints from the signature and it should work.

[varunagrawal]

@GrahamDumpleton any updates on this? :slightly_smiling_face:

[GrahamDumpleton]

Had so many other things to do that haven't been able to give much attention to wrapt. I did get some help in getting test and package building updated, which is why you might see that the diff of your changes is a bit weird as changes for various things you had were made separately.

Anyway, that doesn't address the specific change you are after. I am trying to plan to devote some time to catch up on a lot of stuff on wrapt and get a new release out. I would go through backlog of PRs and issues then so will be able to look properly at this issue then.

Do you think it would be possible to get together a test case for automated testing that doesn't use matplotlib?

[varunagrawal]

@GrahamDumpleton sounds good. I can do that.

After spending some time on this problem, I figured that having the modules in the global namespace was a lot more effort than it was worth, especially since the issue only happens for type annotations. Explicitly removing the type annotations is an easy and generic fix, and solves the problem I originally had.

I'll add a unit test over the weekend if that's okay.

[varunagrawal]

@GrahamDumpleton added the test TestDynamicAdapter::test_adapter_factory_with_type_hints.

[GrahamDumpleton]

The test will need to be in a test file of its own with name tests/test_adapter_py3.py as it can't be run on Python 2.X.

[GrahamDumpleton]

Test will need to be even more selective as to Python version since Iterable type not available for all Python 3.X versions. Else use a different type with test. Anyway, am fixing it up so you don't need to do anything at this point.

[GrahamDumpleton]

Actually, my stuff up, forgot to drop the import from Python 2.X/3.X file when split out Python 3.X code. :-(

[GrahamDumpleton]

I have it all aligned with upstream now and all tests passing. I just need to research Python annotations better now to understand how they work, since have not used them before. Make sure you pull down latest of branch if want to check anything.

[GrahamDumpleton]

As it stands this change is possibly going to need some more work.

The problem I see is exhibited by the code:

def argspec_factory(wrapped):
    return inspect.getfullargspec(wrapped)

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

@annotations_wrapper
def run5():
    return [True]

@annotations_wrapper
def run6() -> Iterable:
    return [True]

print("RUN5")
print(inspect.getfullargspec(run5))
print(run5.__annotations__)

print("RUN6")
print(inspect.getfullargspec(run6))
print(run6.__annotations__)

This produces:

RUN5
FullArgSpec(args=[], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
{}
RUN6
FullArgSpec(args=[], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
{'return': typing.Iterable}

To be consistent I would have thought inspect.getfullargspec(run6) should return annotations value the same as run6.__annotations__.

I need to refresh my memory of what the adapter factory code is meant to do as I actually can't remember. :-)

[GrahamDumpleton]

Have pushed update version, although it still has one outstanding issue to resolve.

The problem case now is:

def func(a, b) -> Iterable:
    return

def argspec_factory(wrapped):
    #return inspect.getfullargspec(wrapped)
    #return inspect.getfullargspec(func)
    return "(a, b) -> Iterable"
    # return func

That is, if providing a signature as string which includes annotations it can fail for same reason as originally if references a type.

[GrahamDumpleton]

I have documented caveat about function signature as string as don't think there is really a solution. Have merged the changes, so let me know if any issues with your use case and can revisit it.

[varunagrawal]

@GrahamDumpleton the changes you made are awesome! Has the CI been updated to publish a new version to PyPI for every merged PR? Or would you have to make a release?

[GrahamDumpleton]

I still have to make manual releases at this point. The CI is building wheels now, but not setup for auto publishing just yet. Before a new release there are still some other old issues trying to address and make changes for. I got today and tomorrow still to work on it.