Skip to content

feat: stubgen warn some modules are ignored#13919

Open
tsbkw wants to merge 3 commits intopython:masterfrom
tsbkw:fix/stubgen-warn-skipping-vendor-dir
Open

feat: stubgen warn some modules are ignored#13919
tsbkw wants to merge 3 commits intopython:masterfrom
tsbkw:fix/stubgen-warn-skipping-vendor-dir

Conversation

@tsbkw
Copy link

@tsbkw tsbkw commented Oct 19, 2022

Print message how many modules ignored when some modules are blacklisted. see: #9599
There are two preceding pull requests exist but no updates since about one year ago.

Suppose these project structure,

a
  __init__.py
  a.py
  vendor
    __init__.py
    a.py

After this change, print messages like this. (line starts with Ignored is added)

$ python -m mypy.stubgen a
Processed 2 modules
Generated files under out/a/
Ignored 2 files

And when specified verbose flag, print message like this. (line starts with Ignored and following the lines are added)

$ python -m mypy.stubgen a -v
Processing 2 files...
Processing a
Created out/a/__init__.pyi
Processing a.a
Created out/a/a.pyi
Processed 2 modules
Generated files under out/a/
Ignored 2 modules
        ignored: a/vendor/__init__.py
        ignored: a/vendor/a.py

Print message how many modules ignored when some modules are
blacklisted. see: python#9599

When option verbose is specified, print ignored modules.
@tsbkw tsbkw marked this pull request as ready for review October 19, 2022 00:59
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks like a good idea to me!
It is better to be explicit about what is not processes and why.

mypy/stubgen.py Outdated
print(f"Generated {files[0]}")
else:
print(f"Generated files under {common_dir_prefix(files)}" + os.sep)
if len(ignored_py_modules):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if len(ignored_py_modules):
if ignored_py_modules:

is enough.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing!
I fixed this, and also clarify the reason why some modules are not processed by aab77a6

os.path.join("pack", "b.py"),
},
)
ignored_files = {os.path.relpath(mod.path or "FAIL") for mod in ignored_py_mods}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is "FAIL"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact I wrote in the same way as files initialized here.

files = {os.path.relpath(mod.path or "FAIL") for mod in py_mods}

StubSource.path may be None, so I guess the original code intend to express failure of module finding and also avoiding error which caused by passing None to os.path.relpath.
Maybe "NOT_FOUND" or just skipping mod without mod.path like below is better. Which do you think is better?

{os.path.relpath(mod.path) for mod in ignored_py_mods if mod.path is not None}

@tsbkw tsbkw requested a review from sobolevn October 26, 2022 00:56
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM, except one question.

I will wait for someone else to double check, because I only have one commit to stubgen. This is not something I am very familiar with.

self.make_file(tmp, "mymodule.py", content="import a")
opts = parse_options(["-m", "mymodule"])
py_mods, c_mods = collect_build_targets(opts, mypy_options(opts))
py_mods, c_mods, ignored_py_mods = collect_build_targets(opts, mypy_options(opts))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignored_py_mods var is not asserted here.
Do we have something to check here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I checked this test case and found its name test_module_not_found is confusing because module is found at this case.

I confirmed that I didn't make any unintentional changes by checking out master branch of mypy, and modified mypy/test/teststubgen.py around L103 like below and run pytest -k StubgenCmdLine -vvv.
The result of the assert_equal(c_mods, []) is fine but assert_equal(py_mods, []) reports error because mymodule was found.

    @unittest.skipIf(sys.platform == "win32", "clean up fails on Windows")
    def test_module_not_found(self) -> None:
        current = os.getcwd()
        captured_output = io.StringIO()
        sys.stdout = captured_output
        with tempfile.TemporaryDirectory() as tmp:
            try:
                os.chdir(tmp)
                self.make_file(tmp, "mymodule.py", content="import a")
                opts = parse_options(["-m", "mymodule"])
                py_mods, c_mods = collect_build_targets(opts, mypy_options(opts))
                assert captured_output.getvalue() == ""
                assert_equal(c_mods, [])
>               assert_equal(py_mods, [])
E               AssertionError: [<mypy.stubgen.StubSource object at 0x113393190>] != []

I fixed test case name and add assertions for variables py_mods, c_mods, ignored_py_mods by 4f44401.

@tsbkw
Copy link
Author

tsbkw commented Oct 27, 2022

Really thank you for reviewing.

I will wait for someone else to double check, because I only have one commit to stubgen. This is not something I am very familiar with.

Totally fine and I will wait for another review 👍

@tsbkw tsbkw requested a review from sobolevn October 27, 2022 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants