Fail to apply typeinfo to a python module in a subdirectory

[bluebird75]

The following will fail :

> python toto\toto.py 
( this generates a type_info.json in current directory )

>dir
[...]
25/07/2018  06:22    <DIR>          toto
25/07/2018  06:21               784 type_info.json

>pyannotate -3 toto\toto.py --type-info type_info.json
No files need to be modified.
NOTE: this was a dry run; use -w to write files

strange, there are type annotations in type_info.json with the correct path

>type type_info.json
[
    {
        "path": "toto\\toto.py",
        "line": 2,
        "func_name": "add",
        "type_comments": [
            "(*int) -> int",
            "(*List[int]) -> List[int]",
            "(*Tuple[int, int]) -> Tuple[int, int]"
        ],
        "samples": 3
    },
    {
        "path": "toto\\toto.py",
        "line": 8,
        "func_name": "add2",
        "type_comments": [
            "(Tuple[int, int], Tuple[int, int]) -> Tuple[int, int, int, int]",
            "(List[int], List[int]) -> List[int]",
            "(int, int) -> int"
        ],
        "samples": 3
    },
    {
        "path": "toto\\toto.py",
        "line": 11,
        "func_name": "main",
        "type_comments": [
            "() -> None"
        ],
        "samples": 1
    }
]

edit the type_info.json to remove the "toto\"

>type type_info.json
[
    {
        "path": "toto.py",
        "line": 2,
        "func_name": "add",
        "type_comments": [
            "(*int) -> int",
            "(*List[int]) -> List[int]",
            "(*Tuple[int, int]) -> Tuple[int, int]"
        ],
        "samples": 3
    },
    {
        "path": "toto.py",
        "line": 8,
        "func_name": "add2",
        "type_comments": [
            "(Tuple[int, int], Tuple[int, int]) -> Tuple[int, int, int, int]",
            "(List[int], List[int]) -> List[int]",
            "(int, int) -> int"
        ],
        "samples": 3
    },
    {
        "path": "toto.py",
        "line": 11,
        "func_name": "main",
        "type_comments": [
            "() -> None"
        ],
        "samples": 1
    }
]

try again

>pyannotate -3 toto\toto.py --type-info type_info.json
Refactored toto\toto.py
--- toto\toto.py        (original)
+++ toto\toto.py        (refactored)
@@ -1,14 +1,18 @@
+from typing import Any
+from typing import List
+from typing import Tuple
+from typing import Union

-def add(*args):
+def add(*args: Any) -> Union[List[int], Tuple[int, int], int]:
     ret = args[0]
     for v in args:
         ret += v
     return v

-def add2(v1, v2):
+def add2(v1: Union[List[int], Tuple[int, int], int], v2: Union[List[int], Tuple[int, int], int]) -> Union[List[int], Tuple[int, int, int, int], int]:
     return v1+v2

-def main():
+def main() -> None:
     print( add(1,2,3) )
     print( add([1,2], [3,4]) )
     print( add((1,2), (3,4)) )
Files that need to be modified:
toto\toto.py
NOTE: this was a dry run; use -w to write files

it worked...

It looks like pyannotate is trimming directories from type_info.json too agressively.

[gvanrossum]

Does it work better if you use forward / on the command line (starting from the top)?

[bluebird75]

Not really. Even if I do that and I convert the backslash to forward slash in type_info.json .

[gvanrossum]

Hm... What if you add an empty __init__.py file to the toto folder? (I'm quite sure that this works at Dropbox.)

[bluebird75]

I tracked down the problem up to here : https://github.com/dropbox/pyannotate/blob/a01510d2960cb8fb0096f6c017391ea9b9e98689/pyannotate_tools/fixes/fix_annotate_json.py#L205

self.__class__.top_dir is 'D:\work\pyann\pyannotate\toto' and it['path'] is 'toto\toto.py' . So joining them together won't work, the directory toto is duplicated in the path.

Looks like crawl_up() was not designed to handle such cases. It miscalculates the top directory in this case : https://github.com/dropbox/pyannotate/blob/a01510d2960cb8fb0096f6c017391ea9b9e98689/pyannotate_tools/fixes/fix_annotate_json.py#L43

[gvanrossum]

Confirmed -- also that if I add an empty __init__.py to the toto folder the problem goes away.

I replicated this by going to the pyannotate project root directory, running python example/driver.py, and then trying pyannotate example/gcd.py.

But I'm not sure what to do about it. It's been too long since I knew this code in and out, and while I know what crawl_up() is supposed to do, I'm no longer sure why that is needed here. If I replace its body with return os.getcwd(), arg the code works, and no tests fail! But I've got a feeling that would break something else (at least in the way we use it at Dropbox) -- I just don't recall what.

[bluebird75]

The code could use more tests in general, and in particular to capture precisely this kind of behavior.

I would expect that this is needed if you are working with multiple in development packages and you are adjusting sys.path to pick all these packages (maybe even with pip). In this case, relating getcwd() current execution dir is no longer the right thing to do.

[jmikedupont2]

I have encountered the same issue and have a small patch based on observations from @gvanrossum that works for me, maybe someone else will benefit from this. For a proper testing there will be more work involved, and I think that a command line option to guide the matching of sources to the json would be good. It could also be embedded in the json collection, so that we find the supposed topdir and embed what we find at collection time into the json that would help the annotator match up.

[andychu]

FWIW I also ran into this, and I just commented out crawl_up() in my local copy for now. For some reason it caused the paths in the items comparison to be one subdirectory off.

Before the change, I had a good type_info.json file, but nothing was being applied. After the change, everything was applied as far as I can tell.

[Safihre]

I am also experiencing the exact same thing. It only works by mofiying the paths in the JSON from:

[
    {
        "path": "C:/Users/Me/Documents/GitHub/sabnzbd3/SABnzbd.py",
        "line": 102,
        "func_name": "GUIHandler",
        "type_comments": [
            "() -> None"
        ],
        "samples": 1
    },
]

to

[
    {
        "path": "SABnzbd.py",
        "line": 102,
        "func_name": "GUIHandler",
        "type_comments": [
            "() -> None"
        ],
        "samples": 1
    },
]

After this it did exactly what I hoped it would do. Thanks very much for this tool ❤️

[gvanrossum]

Unfortunately I have no time available to work on this. If someone could prepare a patch, including a bunch of tests that show how it works in various common and edge cases, I would very much appreciate it.

[jmikedupont2]

Yes I will do it

On Fri, Mar 22, 2019, 11:14 Guido van Rossum notifications@github.com wrote:

Unfortunately I have no time available to work on this. If someone could prepare a patch, including a bunch of tests that show how it works in various common and edge cases, I would very much appreciate it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dropbox/pyannotate/issues/75#issuecomment-475659594, or mute the thread https://github.com/notifications/unsubscribe-auth/APqoaWuMEu0TbiozBy9zwhrlJ2K9MufOks5vZPNGgaJpZM4Vfa9i .

[andychu]

I was thinking it could be as simple as adding a flag like --crawl-up or --no-crawl-up ? As mentioned I could simply delete the crawl_up() call and things work (for me), but that might break some unknown things witin Dropbox?