Fix bug finding implicit namespace packages #1784
Conversation
Not sure how this is passing in CI but failing locally.
...since I never touched this function for anything else.
|
BTW: my comment is still relevant for this to get merged. |
@davidhalter What comment are you referring to? |
|
You can see it above, it's |
|
@davidhalter I don't think there's any visible review comments from you on this PR, are you sure you've submitted them? (If GitHub's UI says it's "Pending" in an orange box I think that means it's pending submission [i.e: a draft comment] rather than pending being addressed). |
There was a problem hiding this comment.
Oh good I'm so sorry. It was really stupid of me to not finish the review.
I really thought that it said "pending", because @bbugyi200 did not change anything.
Thanks for pointing that out @PeterJCLaw.
| def import_module( | ||
| inference_state: "InferenceState", | ||
| import_names: Sequence[str], | ||
| parent_module_value: Any, |
There was a problem hiding this comment.
Please remove the Any hints, they are pretty much useless (and also wrong).
This PR fixes #1759.
To aid in readability and compile-time / lint-time checks, I have also added type signatures to any functions that I have touched.
@davidhalter: I am looking into how to test these changes now. Any pointers on which test files contain the fixtures / helpers that I will probably need?Nevermind, I figured it out: I added some regression tests to thetest_implicit_namespace_package.pytest file. I have also verified that these new tests fail onmasterbut pass on this PR branch.NOTE: I later realized that the
jedi/inference/compiled/subprocess/functions.pyfile did not need any modifications in order to fix this bug; however, since I had already cleaned up that module a bit (e.g. by adding type annotations and removing dead code) and understanding what that module does was probably the biggest hurdle in fixing this bug, I have decided to keep the changes to that module in this PR. If the reviewer disagrees, the changes to that module can be reverted without affecting this bug-fix.