pyre-check icon indicating copy to clipboard operation
pyre-check copied to clipboard

How to resolve importing a function from a module with same name

Open henrylhtsang opened this issue 10 months ago • 6 comments

Hi, in our repo, we have a python file in torchrec/distributed/shard.py that contains a function shard. We also want to keep the shard function inside torchrec/distributed/init.py, which we did by from torchrec.distributed.shard import shard.

However, we got a pyre error torchrec/distributed/init.py:39:0 Undefined import [21]: Could not find a module corresponding to import torchrec.distributed.shard. A definition with that name exists but it's a function.

Any idea on how to resolve?

henrylhtsang avatar Apr 23 '24 02:04 henrylhtsang

Totally understand the frustration. This is a known issue. Unfortunately, the fix will take some time to land, so I would recommend finding a workaround.

migeed-z avatar Apr 23 '24 18:04 migeed-z

@migeed-z thanks, is there a timeline?

henrylhtsang avatar Apr 23 '24 19:04 henrylhtsang

Hi @henrylhtsang.

Unfortunately, there's an architectural issue in Pyre (and a number of other tools too) where we assume that a fully qualified name means something unambiguous in Python, so that torchrec.distributed.shard has to refer to either a module or a function but not both.

The pattern of shadowing a module with a function imported into the parent then causes any tool indexing the code this way to fail to understand it. Until recently, Pyre was failing to analyze the code (which meant it dropped type information) silently, and we recently changed Pyre to warn users that the import can't be analyzed.

This is pretty disruptive, and we also hope we can change Pyre to understand that fully qualified names are not unambiguous and it's necessary to model the effect of an import on scope more precisely, but this will take at least several months for us to fix, possibly longer.

In the meantime, your best bet is probably to rename something. Because you are a library and backward compatibility matters, it's tricky to decide what exactly to do, but:

  • if users typically run from torchrec.distributed import shard then you probably don't want to change the meaning of that, which means you might want to change shard.py to sharding.py or something like that
  • if you're worried that users are also directly importing the module (import torchrec.distributed.shard, or from torchrec.distributed.shard import <something>, then you could potentially also keep shard.py and modify it to just from .sharding import *. At that point, Pyre won't be able to see shard.py but at least at runtime it will be there, so users won't see breakages (unless they are using Pyre)

stroxler avatar Apr 23 '24 20:04 stroxler

(You can of course use a # pyre-ignore to suppress the error you are seeing, but unfortunately users of your library who are using Pyre - which includes most Meta users - will run into the same problem in that case so I wouldn't recommend that approach)

stroxler avatar Apr 23 '24 20:04 stroxler

@stroxler thanks, will explore changing names

henrylhtsang avatar Apr 23 '24 21:04 henrylhtsang

This is a duplicate of https://github.com/facebook/pyre-check/issues/232

vthemelis avatar May 04 '24 15:05 vthemelis