pytype icon indicating copy to clipboard operation
pytype copied to clipboard

Possible name collision for same-named source file in different directories

Open rchen152 opened this issue 6 years ago • 15 comments
trafficstars

From https://github.com/googleapis/google-cloud-python/pull/6769#issuecomment-443837463. That particular issue hasn't been figured out yet, but it got me to think about whether it's generally possible for pytype to accidentally map two source files to the same pyi path, and it indeed is. With a structure like:

root
|----dir1
|    |----foo.py
|----dir2
     |----foo.py

when pytype is run from root, ninja errors out with: ninja: error: build.ninja:11: multiple rules generate /usr/local/google/home/rechen/root/pytype_output/pyi/foo.pyi [-w dupbuild=err]. Perhaps we need to include the relative path from the current directory to the pythonpath directory in pytype_output.

rchen152 avatar Dec 03 '18 19:12 rchen152

note that this is asking pytype to analyse two unrelated projects (that happen to be sitting in the same directory on the user's filesystem). i think we should flag this as an error (possibly with a more informative error message) rather than try to put them in the same output tree.

martindemello avatar Dec 03 '18 20:12 martindemello

True, we may want to forbid this rather than supporting it. Either way, the current behavior (cryptic failure) isn't good.

rchen152 avatar Dec 04 '18 03:12 rchen152

I'm trying to run pytype on a project (https://github.com/cocotb/cocotb) where the same filename exists in two different directory (e.g., https://github.com/cocotb/cocotb/blob/master/cocotb/monitors/avalon.py and https://github.com/cocotb/cocotb/blob/master/cocotb/drivers/avalon.py) - here, this is not unrelated projects. I would therefore be happy if this kind of file layout could be supported.

cmarqu avatar Oct 21 '19 16:10 cmarqu

Hmm, in this case, since all of the packages and subpackages have __init__.py files, I'd have expected pytype to be able to use the full module names to distinguish between the two files. Which directory are you running pytype from in which there's a name collision?

rchen152 avatar Oct 21 '19 18:10 rchen152

I have tried both the very toplevel directory as well as the cocotb subdirectory. Let me boot up the machine where I tried this and see if I can figure out more.

cmarqu avatar Oct 21 '19 20:10 cmarqu

For some reason, I cannot reproduce this anymore. As a workaround, I had added one of those "duplicating" directories to the exclude list, but now that I removed it again, it still works, even after removing the .pytype output directory. Please disregard, sorry for the confusion.

cmarqu avatar Oct 21 '19 21:10 cmarqu

No worries, thanks for looking into it again!

rchen152 avatar Oct 21 '19 23:10 rchen152

An example of this error even within one project is

git clone https://github.com/hill-a/stable-baselines
cd stable-baselines
git checkout 3840ddf752143f7fc1a54bf3debc125855df4ca2
patch -p0 -i patch.diff
pytype -o /tmp/out
pytype -P /tmp/out

where patch.diff contains

diff --git stable_baselines/common/base_class.py stable_baselines/common/base_class.py
index 994a539..e26b591 100644
--- stable_baselines/common/base_class.py
+++ stable_baselines/common/base_class.py
@@ -940,7 +940,7 @@ class ActorCriticRLModel(BaseRLModel):
                              "Stored kwargs: {}, specified kwargs: {}".format(data['policy_kwargs'],
                                                                               kwargs['policy_kwargs']))

-        model = cls(policy=data["policy"], env=None, _init_setup_model=False)
+        model = cls(policy=data["policy"], env=None, _init_setup_model=False)  # pytype: disable=not-instantiable
         model.__dict__.update(data)
         model.__dict__.update(kwargs)
         model.set_env(env)
@@ -1048,7 +1048,7 @@ class OffPolicyRLModel(BaseRLModel):
                              "Stored kwargs: {}, specified kwargs: {}".format(data['policy_kwargs'],
                                                                               kwargs['policy_kwargs']))

-        model = cls(policy=data["policy"], env=None, _init_setup_model=False)
+        model = cls(policy=data["policy"], env=None, _init_setup_model=False)  # pytype: disable=not-instantiable
         model.__dict__.update(data)
         model.__dict__.update(kwargs)
         model.set_env(env)

This gives ninja: error: build.ninja:46: multiple rules generate /__init__.pyi [-w dupbuild=err].

mrahtz avatar Sep 05 '20 13:09 mrahtz

Hmm, why are you running pytype -P /tmp/out? -P is for paths to source code directories, whereas /tmp/out looks like it contains just-generated .imports and .pyi files.

rchen152 avatar Sep 08 '20 18:09 rchen152

I might well have been doing something silly here. I was trying to run pytype in type-check mode, using previously-generated stubs.

Looking through pytype --help, I wasn't sure what -P is supposed to do (why would you specify source code directories with -P rather than just specifying them as the input argument?), but running pytype-single --help, I saw there was a fuller description of -P, and took it to mean that it could be used to specify a directory with previously-generated stubs - and that this also applied to regular pytype. I guess this is not the case?

mrahtz avatar Sep 13 '20 07:09 mrahtz

Ah, the -P arguments to pytype and pytype-single are similar but not quite the same. They're both for telling pytype where the dependencies of your input files are, but for pytype, you specify source files whereas for pytype-single you specify type stubs. You wouldn't just pass these files as inputs because they would then be type-checked.

rchen152 avatar Sep 16 '20 02:09 rchen152

Ahh, gotcha :) Thanks!

mrahtz avatar Sep 16 '20 08:09 mrahtz

This reproduces every time for me if there's no __init__.py in the folder of one of the duplicates.

nirradi avatar Oct 06 '21 08:10 nirradi

Agree with @nirradi that adding a missing __init__.py fixed the problem for me. To future visitors to this page: you may be missing __init__.py files somewhere in your packages.

Maybe the multiple rules generate error should be extended to include a note that pytype has poor compatibility with PEP 420 implicit namespace packages and to try adding __init__.py files?

lucaswiman avatar Sep 23 '22 06:09 lucaswiman

Aha, knowing that adding __init__.py files fixes the problem is helpful, thanks! It sounds like the issue is probably here: https://github.com/google/pytype/blob/173a7a416787a1b37f4a69ac6d00e18c69788b3e/pytype/tools/environment.py#L71-L78. We're relying on the presence of __init__.py to find subpackages, which indeed does not work for implicit namespace packages.

rchen152 avatar Sep 27 '22 06:09 rchen152