pub.subsribe fails when using python 3 typehints in callable

[superdweebie]

Here is the error log: File "C:\Users\J84375\AppData\Local\Programs\Python\Python36\lib\site-packages\pubsub\core\publisher.py", line 160, in subscribe subscribedListener, success = topicObj.subscribe(listener, **curriedArgs)

File "C:\Users\J84375\AppData\Local\Programs\Python\Python36\lib\site-packages\pubsub\core\topicobj.py", line 353, in subscribe args, reqd = topicArgsFromCallable(listener, ignoreArgs=curriedArgs)

File "C:\Users\J84375\AppData\Local\Programs\Python\Python36\lib\site-packages\pubsub\core\topicargspec.py", line 51, in topicArgsFromCallable argsInfo = getListenerArgs(_callable, ignoreArgs=ignoreArgs)

File "C:\Users\J84375\AppData\Local\Programs\Python\Python36\lib\site-packages\pubsub\core\callables.py", line 221, in getArgs return CallArgsInfo(func, firstArgIdx, ignoreArgs=ignoreArgs)

File "C:\Users\J84375\AppData\Local\Programs\Python\Python36\lib\site-packages\pubsub\core\callables.py", line 147, in __init__ (allParams, varParamName, varOptParamName, defaultVals) = getargspec(func)

File "C:\Users\J84375\AppData\Local\Programs\Python\Python36\lib\inspect.py", line 1072, in getargspec

raise ValueError("Function has keyword-only parameters or annotations" ValueError: Function has keyword-only parameters or annotations, use getfullargspec() API which can support them

Removing the type hints from the callable signature removes the error. However, it would be very nice to use typehints!

[skewty]

@schollii I believe I have a small code patch that fixes this issue. Please see below:

from inspect import ismethod, isfunction, signature, Parameter
# removed getargspec and added signature and Parameter to import

class CallArgsInfo:
    def __init__(self, func: UserListener, firstArgIdx: int, ignoreArgs: Sequence[str] = None):
        allParams = []
        defaultVals = []
        varParamName = None
        varOptParamName = None
        for argName, param in signature(func).parameters.items():
            if param.default != Parameter.empty:
                defaultVals.append(param.default)
            if param.kind == Parameter.VAR_POSITIONAL:
                varParamName = argName
            elif param.kind == Parameter.VAR_KEYWORD:
                varOptParamName = argName
            else:
                allParams.append(argName)

        # code below is unchanged

        self.acceptsAllKwargs = (varOptParamName is not None)
        self.acceptsAllUnnamedArgs = (varParamName is not None)

[schollii]

Hi skewty thanks for submitting this. Sorry I didn't reply sooner, somehow I didn't get a notification of your initial post and missed it when looked.

[JoshMayberry]

@schollii I kept getting the same error, but it was because I was trying to call a function that only had keyword arguments. The method provided by @skewty was not able to handle it either. Below is a patch I created to fix this problem.

The big change was (1) using getfullargspec instead of getargspec, and (2) placing the values for the args and kwargs in dictionaries where the key is the variable name and the value is its default. This is needed because args and kwargs can both have default values.

from inspect import getfullargspec
class CallArgsInfo:
    """
    Represent the "signature" of a listener of topic messages: which arguments are
    required vs optional.
    """

    class NO_DEFAULT:
        def __repr__(self):
            return "NO_DEFAULT"

    def __init__(self, func: UserListener, firstArgIdx: int, ignoreArgs: Sequence[str] = None):
        """
        :param func: the callable for which to get paramaters info
        :param firstArgIdx: 0 if listener is a function, 1 if listener is a method
        :param ignoreArgs: do not include the given names in the getAllArgs(), getOptionalArgs() and
            getRequiredArgs() return values

        After construction,
        - self.allParams will contain the subset of 'args' without first
          firstArgIdx items,
        - self.numRequired will indicate number of required arguments
          (ie self.allParams[:self.numRequired] are the required args names);
        - self.acceptsAllKwargs = acceptsAllKwargs
        - self.autoTopicArgName will be the name of argument
          in which to put the topic object for which pubsub message is
          sent, or None. This is identified by the argument that has a
          default value of AUTO_TOPIC.

        For instance,
        - listener(self, arg1, arg2=AUTO_TOPIC, arg3=None) will have self.allParams = (arg1, arg2, arg3),
            self.numRequired=1, and self.autoTopicArgName = 'arg2', whereas
        - listener(self, arg1, arg3=None) will have self.allParams = (arg1, arg3), self.numRequired=1, and
            self.autoTopicArgName = None.
        """

        args, varParamName, varOptParamName, argsDefaults, kwargs, kwargsDefaults, annotations = getfullargspec(func)

        self.allArgs = {}

        if(argsDefaults != None):
            argsDefaults_startsAt = len(args) - len(argsDefaults) - 1
        for i, variable in enumerate(args):
            if ((i == 0) and (firstArgIdx > 0)):
                continue #skip self

            if ((argsDefaults == None) or (i < argsDefaults_startsAt)):
                self.allArgs[variable] = self.NO_DEFAULT()
            else:
                self.allArgs[variable] = argsDefaults[i - argsDefaults_startsAt - 1]

        self.allKwargs = {}
        for variable in kwargs:
            if ((kwargsDefaults == None) or (variable not in kwargsDefaults)):
                self.allKwargs[variable] = self.NO_DEFAULT()
            else:
                self.allKwargs[variable] = kwargsDefaults[variable]

        self.acceptsAllKwargs = (varOptParamName is not None)
        self.acceptsAllUnnamedArgs = (varParamName is not None)
        self.allParams = [*self.allArgs.keys(), *self.allKwargs.keys()]

        if ignoreArgs:
            for var_name in ignoreArgs:
                if (var_name in self.allArgs):
                    del self.allArgs[var_name]
                elif (var_name in self.allKwargs):
                    del self.allKwargs[var_name]

            if (varOptParamName in ignoreArgs):
                self.acceptsAllKwargs = False
            if (varParamName in ignoreArgs):
                self.acceptsAllUnnamedArgs = False

        self.numRequired = sum([1 for value in [*self.allArgs.values(), *self.allKwargs.values()] if (isinstance(value, self.NO_DEFAULT))])
        assert self.numRequired >= 0

        # if listener wants topic, remove that arg from args/defaultVals
        self.autoTopicArgName = None
        self.__setupAutoTopic()

    def getAllArgs(self) -> List[str]:
        return tuple(self.allParams)

    def getOptionalArgs(self) -> List[str]:
        return tuple([key for key, value in [*self.allArgs.items(), *self.allKwargs.items()] if (not isinstance(value, self.NO_DEFAULT))])

    def getRequiredArgs(self) -> List[str]:
        """
        Return a tuple of names indicating which call arguments
        are required to be present when pub.sendMessage(...) is called.
        """
        return tuple([key for key, value in [*self.allArgs.items(), *self.allKwargs.items()] if (isinstance(value, self.NO_DEFAULT))])

    def __setupAutoTopic(self) -> int:
        """
        Does the listener want topic of message? Returns < 0 if not,
        otherwise return index of topic kwarg within args.
        """
        for variable, value in {**self.allArgs, **self.allKwargs}.items():
            if (value == AUTO_TOPIC):
                del self.allArgs[variable]
                return

Here are some test cases I made that are now able to pass:

def test_1(arg1 = 1, arg2 = 2, *args, kwarg1, kwarg2): print(arg1, arg2, args, kwarg1, kwarg2)
def test_2(arg1, arg2, arg3, *args, kwarg1 = 1, kwarg2 = 2, **kwargs): print(arg1, arg2, arg3, args, kwarg1, kwarg2, kwargs)
def test_3(kwarg1 = 1, kwarg2 = 2): print(kwarg1, kwarg2)
def test_4(): print(None)
class Test():
    def test(self, arg1, arg2 = 1): print(self, arg1, arg2)
test_5 = Test()

pubsub.pub.subscribe(test_1, 'example_1')
pubsub.pub.sendMessage('example_1', kwarg1 = 123, kwarg2 = 456)
pubsub.pub.subscribe(test_2, 'example_2')
pubsub.pub.sendMessage('example_2', arg1 = 123, arg2 = 456, arg3 = 789)
pubsub.pub.subscribe(test_3, 'example_3')
pubsub.pub.sendMessage('example_3', kwarg2 = 456)
pubsub.pub.subscribe(test_4, 'example_4')
pubsub.pub.sendMessage('example_4')
pubsub.pub.subscribe(test_5.test, 'example_5')
pubsub.pub.sendMessage('example_5', arg1 = 123)

[schollii]

For sure, I will try to address several tickets upon return from vacation July 8.

[schollii]

@JoshMayberry any chance you could submit a PR? (including a new test class with your 5 tests)

[JoshMayberry]

@schollii I would be happy to create a test class that shows the five tests. I do not know what a 'PR' is though. Can you elaborate on that?

[skewty]

A PR is a Pull Request.

If you are not familiar with collaboration on GitHub / GitLab the process is basically: you create a branch of the source code (this is now a private copy for you). You make the software code changes that "fix" or add the feature you are looking for. You then assemble the software code changes into a "pull request". The developers can look at the software code changes you made and if they look acceptable, they will "accept" them. Which then makes your software code part of the code that you originally "branched" off of.

+Scott

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On 9 July 2018 7:40 AM, Josh notifications@github.com wrote:

@schollii I would be happy to create a test class that shows the five tests. I do not know what a 'PR' is though. Can you elaborate on that?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[schollii]

@JoshMayberry More details:

1. From github, fork pypubsub; you can then see in your own account that you have a copy
2. On your dev machine, checkout a new branch, calling it whatever you want (typically something related to the feature/fix)
3. Modify, commit, push code (as many times as you want)
4. When you are satisfied with the mods, then from your github, create a pull request
5. I will receive notification of a pull request, I'll have a look at the modified files and flag any issues that need fixing. If there are any, you'll make fixes on your branch and the PR will keep track of such changes and I can see them (I don't think a new PR is needed -- on bitbucket it is not, but I haven't had to deal with this situation on github)
6. When all is good, I merge and close your PR

[JoshMayberry]

@skewty Thanks for explaining what a PR is. @schollii Thanks for the step-by-step instructions for how submitting a change works.

I created a pull request with the changes I have made, along with a test file. Unfortunately, I don't think the test cases are structured the same way you do your test cases, so I'm sure it will have to be modified to match the way you do it. For now, it uses the module unittest.

[schollii]

@JoshMayberry Important to ensure that no other tests fail as a result of these changes, which go pretty deep so need to be careful. How to run the test suite is in the docs IIRC, let me know if you have issues (it's quite straightforward: pip install pytest, then change one import, remove the "if main", and that should be pretty much it).

[schollii]

I'm hoping I can figure this out soon, just have not had time to look into it.

[schollii]

Thanks @JoshMayberry for bringing this up and for trying to fix this, your effort was much appreciated. Please try out the new version (download from github and install) and let me know if it works in your app.

If you take a look at that callables.py you will notice the solution was in fact to simplify the code with inspect.signature/Parameters, the keyword-only args were then automatically covered (you'll notice there is nothing in the fix specific to keyword only args!). This is not surprising as conceptually, keyword-only args are irrelevant to pypubsub: pypubsub enforces keyword-only args in sendMessage, thus the real issue was the assumption that the old code was making: that default values were only for keyword args. Once I removed that assumption, the code was a lot simpler and de-factor covered the keyword-only case. The code complexity was a result of gradual changes over time that never got properly examined. Pretty cool.

I'm closing this, but please re-open a separate ticket if you run into issues. I think I will release 4.0.1.