[package] Introduce BaseImporter to manage module namespace collisions.

[suo]

Stack from ghstack:

• [fx/package] make GraphModules packageable #51976 [fx/package] make GraphModules packageable
• [fx] Separate globals assignment from code generation. #51974 [fx] Separate globals assignment from code generation.
• [package] Introduce BaseImporter to manage module namespace collisions. #51975 [package] Introduce BaseImporter to manage module namespace collisions.
• [package] rename ex/importer.py to package_ex/importer.py #52320 [package] rename ex/importer.py to package_ex/importer.py

See comments in code.

Differential Revision: D26340592

[facebook-github-bot]

💊 CI failures summary and remediations

As of commit 1bf3670 (more details on the Dr. CI page):

---

• 5/5 failures possibly* introduced in this PR
  • 3/5 non-scanned failure(s)

---

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_linux_xenial_py3_clang7_onnx_ort_test1 (1/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Error: No such container:path: d36760ab16b7c2078caa57594303a4007696cc14a8f633e29e4ee39acfaae8c8:/var/lib/jenkins/workspace/test/.coverage

Feb 19 01:22:58     * d903106bad          0 reports
Feb 19 01:22:58     * 3978ffb37a          0 reports
Feb 19 01:22:58     |
Feb 19 01:22:58     :
Feb 19 01:22:58 
Feb 19 01:22:58 Removed  (across    0 suites)     0 tests, totaling     0.00s
Feb 19 01:22:58 Modified (across    0 suites)     0 tests, totaling     0.00s
Feb 19 01:22:58 Added    (across    0 suites)     0 tests, totaling     0.00s
Retrieving test reports
Retrieving Python coverage report
Error: No such container:path: d36760ab16b7c2078caa57594303a4007696cc14a8f633e29e4ee39acfaae8c8:/var/lib/jenkins/workspace/test/.coverage


Exited with code exit status 1

pytorch_linux_xenial_py3_clang7_onnx_ort_test2 (2/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Error: No such container:path: f30409a1a803c24de381ca21e58966b04e152a781be1d6355e5165c587010e1d:/var/lib/jenkins/workspace/test/.coverage

Feb 19 01:22:19     * d903106bad          0 reports
Feb 19 01:22:19     * 3978ffb37a          0 reports
Feb 19 01:22:19     |
Feb 19 01:22:19     :
Feb 19 01:22:19 
Feb 19 01:22:19 Removed  (across    0 suites)     0 tests, totaling     0.00s
Feb 19 01:22:19 Modified (across    0 suites)     0 tests, totaling     0.00s
Feb 19 01:22:19 Added    (across    0 suites)     0 tests, totaling     0.00s
Retrieving test reports
Retrieving Python coverage report
Error: No such container:path: f30409a1a803c24de381ca21e58966b04e152a781be1d6355e5165c587010e1d:/var/lib/jenkins/workspace/test/.coverage


Exited with code exit status 1

---

ci.pytorch.org: 2 failed

• Failed: pr/pytorch-linux-bionic-rocm4.0.1-py3.6
• Failed: pr/caffe2-pytorch-linux-bionic-rocm4.0.1-py3.6-test

---

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

[zdevito]

I have a bunch of somewhat disparate minor comments to try to clean up the API.

[zdevito]

Cool! Just a couple minor things inline.

[zdevito]

Because this is the start of this function, and because it is only called in one place from our custom pickler, I think it should go in that function and not here. I can see this accidentally firing for weird objects in other places.

[zdevito]

Does that make sense or am I not understanding what the string version of reduce does?

[suo]

afaict we need this here, because __reduce__ -> str is one way that objects can signal that they want to be global'd as a different name than what the pickler might infer. I ran across this when trying to package references to typing.List and friends in FX.

We can put equivalent logic in the import string formatter in GraphModule, but it made more sense to put it here and leave clients of the code unaware of this detail.

[zdevito]

ok, I get it now. Given that definition of what it does, then it is in the right place.

[zdevito]

we don't need it for this patch, but if you catch a ObjMismatchError here, then you can keep looking for the object, and if you succeed, you can report that the earlier importer shadowed the importer that you needed.

[suo]

Yeah good idea. Right now we use the package mangling as an implementation detail to give a good message, but it would be better to do it without that knowledge.

[facebook-github-bot]

@suo merged this pull request in d5ac929.