Stubgen : Adding a print message on ignoring module <path>

[noob8boi]

#9599

Description

I added a print command in <is_blacklisted_path> function which shows a message of ignoring module from BLACKLIST. I am really sorry if this PR is not helpful.
(Explain how this PR changes mypy.)

Test Plan

I am pretty new to open source contribution and I don't know if it's necessary to test a print command...

[noob8boi]

What are these checks about?

[JelleZijlstra]

Looks like the tests are checking the stdout from stubgen, and failing because of the new print statement.

[hauntsaninja]

Thanks, and welcome to here!

I think the print should be added to remove_blacklisted_modules, as it stands the change is just going to print every module that goes through stubgen.

[noob8boi]

Oh ok. I will try that .

[noob8boi]

I am not sure why the checks are failing again.. I tried doing

def remove_blacklisted_modules(modules: List[StubSource]) -> List[StubSource]:
    module = [module for module in modules]
    if is_blacklisted_path(module.path):
        print(f"Ignoring Module Path '{module.path}'")
    elif module.path is None or not is_blacklisted_path(module.path):
        return module

Am I doing something wrong?

[SwagatSBhuyan]

I am not sure why the checks are failing again.. I tried doing

def remove_blacklisted_modules(modules: List[StubSource]) -> List[StubSource]:
    module = [module for module in modules]
    if is_blacklisted_path(module.path):
        print(f"Ignoring Module Path '{module.path}'")
    elif module.path is None or not is_blacklisted_path(module.path):
        return module

Am I doing something wrong?

On running these changes on my local system, I got a few errors:

$ python stubgen.py
Traceback (most recent call last):
  File "stubgen.py", line 1691, in <module>
    main()
  File "stubgen.py", line 1687, in main
    generate_stubs(options)
  File "stubgen.py", line 1545, in generate_stubs
    py_modules, c_modules = collect_build_targets(options, mypy_opts)
  File "stubgen.py", line 1321, in collect_build_targets
    py_modules = remove_blacklisted_modules(py_modules)
  File "stubgen.py", line 1275, in remove_blacklisted_modules
    if is_blacklisted_path(module.path):
AttributeError: 'list' object has no attribute 'path'

Also, modules in itself is a list, so making another list called module is not necessary I guess. Moreover, the list module that you just made will not have any attribute named path, rather its elements, i guess there needs to be a wrapper loop.

I'll be trying to implement these changes as well. See if I can come around these issues.

[noob8boi]

Btw I am supposed to return a list of modules which pass the if condition, right? Also in the error it says that m.path was expecting str instead of Optional[str] .. in the main code u were able to pass the Optional[str] .. sorry if I am making it worse.

[pranavrajpal]

in the main code u were able to pass the Optional[str]

The original version avoided calling is_blacklisted_path when m.path was None. Your code doesn't do that, which is why mypy is giving you an error.

[noob8boi]

in the main code u were able to pass the Optional[str]

The original version avoided calling is_blacklisted_path when m.path was None. Your code doesn't do that, which is why mypy is giving you an error.

Thank you . I will try to fix it further :)

[noob8boi]

YES FINALLY THE CHECKS PASSED.. LESGOOO

[terencehonles]

@Infinil following up from my comment on #11493 (comment) with one nit picky thing you can change, your PR title: We all know that it's "stubgen.py" (you're working on a Python codebase) but you've spelt that as "Stubgen py :", but the name for the command does not generally include ".py" so you can just call it "stubgen". You are also describing that your printing a message, but then also saying why you are printing that message. You can leave out that you're printing a message, but just say what you're printing/when you're printing, and it looks like you accidentally capitalized "Ignoring".

As I mentioned, this is all nit picky and your PR title is fine, but since you said you are new and want to learn I'm trying to help 🙂

This is the example PR titles I would suggest:

• "stubgen: print ignored module paths"
• "stubgen: print skipped module paths"
• "Print module paths stubgen ignores"

Feel free to use any of these or come up with your own 🙂 (or just keep yours how it is if you really don't want to change it)

[noob8boi]

@Infinil following up from my comment on #11493 (comment) with one nit picky thing you can change, your PR title: We all know that it's "stubgen.py" (you're working on a Python codebase) but you've spelt that as "Stubgen py :", but the name for the command does not generally include ".py" so you can just call it "stubgen". You are also describing that your printing a message, but then also saying why you are printing that message. You can leave out that you're printing a message, but just say what you're printing/when you're printing, and it looks like you accidentally capitalized "Ignoring".

As I mentioned, this is all nit picky and your PR title is fine, but since you said you are new and want to learn I'm trying to help 🙂

This is the example PR titles I would suggest:

• "stubgen: print ignored module paths"
• "stubgen: print skipped module paths"
• "Print module paths stubgen ignores"

Feel free to use any of these or come up with your own 🙂 (or just keep yours how it is if you really don't want to change it)

Thank you for the help 🙂 .. btw should I close with comment or wait for a reviewer?

[terencehonles]

@Infinil just wait. They're probably busy 😉

[JukkaL]

Sorry for the slow review response! The PR looks reasonable to me, but it has some conflicts. @noob8boi are you interested in fixing the conflicts? Somebody else can also help finish this PR.