Partially-annotated functions with default parameter values result in errors.

[sjjessop]

In this code, the only difference between Foo and Bar is whether or not the parameter a has a type annotation:

from typing import Any

from pytypes import typechecked

class Foo:
    def log(self, a: Any, b: Any = None) -> None:
        print(self, a, b)

class Bar:
    def log(self, a, b: Any = None) -> None:
        print(self, a, b)

if __name__ == '__main__':
    typechecked('__main__')
    Foo().log(1)
    Bar().log(2)

The result is:

$ python test.py
<__main__.Foo object at 0x000002871A85E978> 1 None
Traceback (most recent call last):
  File "test.py", line 20, in <module>
    Bar().log(2)
pytypes.exceptions.InputTypeError:
  __main__.Bar.log
  called with incompatible types:
Expected: Tuple[Any, Union[Any, NoneType], NoneType]
Received: Tuple[int, NoneType]

That is, the type checking is happy with Foo, but with Bar it seems to expect an extra None argument after b.

The problem goes away when b doesn't have a default value. It doesn't go away when a second argument is added to the call to Bar().log.

[Stewori]

I can confirm this is a bug. Note that it is OOP specific. If not in classes, the same scenario appears to work as expected:

def log1(a: Any, b: Any = None) -> None:
    print(a, b)

def log2(a, b: Any = None) -> None:
    print(a, b)

if __name__ == '__main__':
    typechecked('__main__')
    log1(1)
    log2(2)

Thanks for spotting this. However I am currently not able to work on pytypes, so this won't be fixed anytime soon (unless someone steps in and makes a PR).

[Stewori]

Another thing occurred to me which should be tracked here: Todo: Check if this also affects @staticmethod, @classmethod and @property.

[Stewori]

The cause is somewhere in type_util._funcsigtypes.

[Stewori]

The issue is that type_util._handle_defaults is not self-aware.

[Stewori]

I think I fixed it on my local machine, but writing tests etc properly takes time. It would be helpful if you could rewrite your code sample above as a test (including @staticmethod etc cases). However until this is merged, let me point out a workaround: You can turn off default-inference by pytypes.infer_default_value_types = False I checked that this "cures" the issue.

[sjjessop]

Is it enough to have tests of using @typechecked to decorate the various kinds of methods, or would you also need tests of calling it on a module (as in the code I originally reported), and/or tests of the underscored functions in type_util?

[Stewori]

First of all, apologize that the test suite is such a mess. I know it needs refactoring but there are always other priorities. The test for this should go next to https://github.com/Stewori/pytypes/blob/master/tests/test_typechecker.py#L4454 but as a separate test method. Link this issue like it is frequently done, e.g. in https://github.com/Stewori/pytypes/blob/master/tests/test_typechecker.py#L4762. Python 3 syntax i.e. the helper class in this case, must go into https://github.com/Stewori/pytypes/blob/master/tests/testhelpers/typechecker_testhelper_py3.py, because test_typechecker still contains Python 2 tests (Python 2 support is kept for Jython). I think there should only one class be required for this test.

Is it enough to have tests of using @typechecked to decorate the various kinds of methods

Yes. Ideally, simply decorate the helperclass with @typechecked, that will apply to all methods. Testing of the _ underscore methods is not required (I know it should be but this is not the time to revolutionize the test suite, see #11 ). For now I am fine with just a single symptomatical test that pins this issue, close to your initial example + staticmethod and classmethod cases if they should be affected by this issue (didn't check yet). (With "pin" I mean the test code should fail before the fix and pass after the fix. You can check this using pytypes.infer_default_value_types = False)

If you prefer it, you can also just post the test code here and I'll put it into the right places. OTOH if you file a proper PR you will be credited in the project git history.

[sjjessop]

No problem, the only difficulty I had with the test suite was that I didn't leave enough blank lines after my helper class, which caused a completely unrelated test to fail. Stupid comment-based annotations ;-)

@classmethod and @property.setter are affected, @staticmethod isn't (probably no suprises there). Shall I put a staticmethod test in there anyway, for completeness, or do you prefer to have only the tests for this issue?

[Stewori]

Re testing staticmethod. I am undecided. Do it like you prefer.

[sjjessop]

PR build fails on Travis in the way I expected (Python 2.7 and 3.4 pass because the tests are skipped. Python 3.5+ three tests fail). You have the permissions to make the fix in my fork before merging, if you prefer.

[sjjessop]

While I'm in here, are you interested in PRs for either of the following minor niggles? I won't be offended if you aren't!

• Typo in the readme: "It's main features are currently" should have no apostrophe.
• Quite a bit of trailing whitespace in both the files I edited. I only noticed because I have my text editor set to trim trailing whitespace on save, which created a messy diff.

[Stewori]

Thanks for the help! Please go ahead.