Repository navigation
fix: Fix import paths with site-packages as part of package name - #756
last-partizan wants to merge 4 commits into
Conversation
| pass | ||
| else: | ||
| # If path includes "site-packages", we're interested in part after this. | ||
| rel_path_parts = rel_path_parts[site_packages_index + 1:] |
There was a problem hiding this comment.
That's temporary code from my first attempt, but we probably need to leave here a check for no "site-packages" in path. Just in case.
| extra_ignores = [ | ||
| p.pathlib.relative_to(self.root.pathlib) | ||
| for p in self.get_python_path_folders() | ||
| if p.pathlib.is_relative_to(self.root.pathlib) | ||
| and "site-packages" in p.pathlib.parts | ||
| ] | ||
| for p in extra_ignores: | ||
| # I'm using this approach, because self.prefs.add after _init_prefs | ||
| # does not work for some reason. | ||
| prefs["ignored_resources"].append(str(p)) |
There was a problem hiding this comment.
That's my second attempt, and i need feedback here.
Is it good approach, or maybe some other ideas?
BuiltinModule had no get_scope, so consumers gating on rope's module semantics (flext-infra's RopeSemanticModule protocol) rejected every C/builtin module; BuiltinModuleScope now serves the live attribute mapping as a Module-kind scope. The python-rope#756 site-packages ignore preflight also crashed when callers did not preseed the ignored_resources preference; it now seeds the key itself. Both behaviors carry regression tests (2940 total passed).
|
#814 proposes a related approach: classify packages under ignored resources, including an in-project virtualenv, as non-project packages for autoimport purposes. I would prefer to build on that resource-classification approach rather than add a separate filter for paths containing site-packages. However, these changes touch different parts of autoimport, so we should not assume #814 fixes your original case. Could you turn the original virtualenv layout into a regression test and try it against #814? If it still produces incorrect module names, please share the remaining failure so we can determine whether an additional discovery fix is needed. Please keep this draft open until that reproduction is covered. |
Description
This is another fix, related to #722, i just started using
python-lsp-serverandropefor autoimports, and noticed it includes.direnv.python-3.11.lib.python3.11as part of module name when importing.We need to cut part of the path to site-packages, to generate correct import name.That was my first attempt to fix this, but turned that wasn't a problem. The problem was, in my case - that local venv
.venvwas included in project resources.As far as i understand, we have two caches
generate_cachecreates project-specific cachegenerate_module_cachecreates cache of system/venv modules.And when .venv is in the same dir as a project, it was included in project files.
My current first attempt just filters out path if it has "site-packages" in it, but, maybe we need smarter solution, for example:
get_python_filesshould exclude any file fromget_python_path_folders(except project root).For this i need some guidance, @tkrabel @lieryan
Checklist (delete if not relevant):