Fixed IndexError when pytypes splits filepaths in code that warns about argument names

[lubieowoce]

Hi again, I've got another one! (Python 3.5 inside pipenv on Windows)

Note that setting pytypes.warn_argnames = False avoids this.

In a file preview_output.py, I have code that looks something like this:

@typechecked
class Thing:
    @classmethod
    def Foo(_cls, x: int, y: int):
        ...
    ...

f = Thing.Foo(3, 5)

(I have reasons for _cls to have that leading underscore, but that's beside the point) But running preview_output.py gives me this error:

Traceback (most recent call last):
  File "preview_output.py", line 40, in <module>
    f = Thing.Foo(3, 5)
  File "c:\users\lubieowoce\documents\pytypes\pytypes\typechecker.py", line 771, in checker_tp
    func0, slf, clsm)
  File "c:\users\lubieowoce\documents\pytypes\pytypes\util.py", line 799, in _warn_argname
    off = _calc_traceback_list_offset(tb)
  File "c:\users\lubieowoce\documents\pytypes\pytypes\util.py", line 780, in _calc_traceback_list_offset
    if tb_list[off][0].split(os.sep)[-2] == 'pytypes':
IndexError: list index out of range

After inspection, I found out tb_list[off][0] is equivalent¹ to tb_list[off].filename. In my case, tb_list[off].filename is just 'preview_output.py', so splitting it on os.sep gives ['preview_output.py'] which has no [-2], hence the IndexError. I'm guessing that on other platforms / in other circumstances traceback() gives absolute filepaths (that's what inspect.stack() does on my machine) and no one hit this error yet. Anyway, I just added a length check (and a comment explaining what it's supposed to do) and I think it should work as intended. Tell me what you think!

¹ ...at least in Python 3.5 – were you using [0] for compatibility reasons?

[Stewori]

This one is a bit difficult because we currently don't have tests for checking the error output formats (something like doctests would be needed I suppose). That means it is hard to assert that nothing else gets broken. Anyway, this is fixing a crash so I definitely like to accept this. Do you see a chance to provide a test here?

[lubieowoce]

Sure, I'll get on it later today.

[lubieowoce]

Update: It turns out it's not that easy to write tests for it, especially because I don't know what causes traceback to use a relative path for the user module in some cases (the cause of my original issue, and the motivation for this PR). Also, I couldn't find a nice way to obtain a deep traceback from within pytypes' internals, so I can't test the traceback-trimming. I came up with a bunch of potential solutions (tldr - option 3 seems the nicest)

1. Add a dummy function that just returns the current traceback to the main pytypes package. We then have a way to get a "realistic" traceback on which traceback-trimming functions can be tested. Kinda ugly, but should work. (maybe we could avoid cluttering up the main code by injecting the dummy function dynamically at test time?)

2. Create a bunch of mock tracebacks, since they're just lists of namedtuples with a bunch of strings inside. This frees us from depending on the filesystem and gives us more control – we can simulate any scenario we like. But, the other side of the coin is that we have to trust that the mock tracebacks reflect reality.

3. Emulate the real scenario – disable pytypes' "nice tracebacks" option; make a @typechecked class with a method that'll cause a warning; intercept the warning object like this¹; and extract the traceback object. This one is the most "realistic", but the tracebacks would all originate from the tests folder – we can't test as many scenarios as with option 2. (perhaps combining the two approaches makes sense?)

I'll try to make something like that work tomorrow, let me know what you think!

EDIT: Just a vague idea: maybe sys.excepthook could work for checking exception tracebacks like the catching-warnings approach from option 3?

[Stewori]

I told you it's hard to test. But don't spend too much effort. I can accept this without a test. However I would have to review much more carefully and assert somewhat manually that original use cases are not broken. I don't know when I'll find time for this.

[Stewori]

Sorry for delaying this so long. I wasn't able to work on pytypes for a while. This issue looks similar to the one fixed in https://github.com/Stewori/pytypes/commit/0a90a4ca52c0dca0609b9fc963f000695de07283 i.e. #51. Actually I should have fixed it here too but overlooked that the same kind of code is repeated there. I think both solutions work - your's is shorter, mine is a bit safer I suppose. It's mainly cosmetical considerations that I think it should be fixed in the same manner both in _calc_traceback_limit and in _calc_traceback_list_offset. However I am fine with accepting this PR for now but will probably edit the code later to put both occurrences into the same shape (your's or mine, I didn't decide yet).

[Stewori]

Another note: You may want to set pytypes.warn_argnames = False.