-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autoimport autodetection #516
base: master
Are you sure you want to change the base?
Changes from 9 commits
310fafe
c498523
1317549
8650ff6
e9fa30d
a05f7d5
0a94b92
ef1c0a8
ef237d4
5fb158b
f2fd9e6
97cfe93
5b78bb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,14 @@ | |
from rope.base.resources import Folder | ||
|
||
|
||
@dataclass | ||
class AutoimportPrefs: | ||
underlined: bool = field( | ||
default=False, description="Cache underlined (private) modules") | ||
memory: bool = field(default=None, description="Cache in memory instead of disk") | ||
parallel: bool = field(default=True, description="Use multiple processes to parse") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about this further on the config. A couple thoughts:
|
||
|
||
|
||
@dataclass | ||
class Prefs: | ||
"""Class to store rope preferences.""" | ||
|
@@ -206,6 +214,8 @@ class Prefs: | |
Can only be set in config.py. | ||
"""), | ||
) | ||
autoimport: AutoimportPrefs = field( | ||
default_factory=AutoimportPrefs, description="Preferences for Autoimport") | ||
|
||
def set(self, key: str, value: Any): | ||
"""Set the value of `key` preference to `value`.""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,10 @@ | |
from threading import local | ||
from typing import Generator, Iterable, Iterator, List, Optional, Set, Tuple | ||
|
||
from packaging.requirements import Requirement | ||
from copy import deepcopy | ||
from rope.base import exceptions, libutils, resourceobserver, taskhandle, versioning | ||
from rope.base.prefs import AutoimportPrefs | ||
from rope.base.project import Project | ||
from rope.base.resources import Resource | ||
from rope.contrib.autoimport import models | ||
|
@@ -53,18 +56,36 @@ def get_future_names( | |
|
||
|
||
def filter_packages( | ||
packages: Iterable[Package], underlined: bool, existing: List[str] | ||
packages: Iterable[Package], | ||
underlined: bool, | ||
existing: List[str], | ||
dependencies: Optional[List[Requirement]], | ||
) -> Iterable[Package]: | ||
"""Filter list of packages to parse.""" | ||
parsed_deps = ( | ||
[dep.name for dep in dependencies] if dependencies is not None else None | ||
) | ||
|
||
def is_dep(package) -> bool: | ||
return ( | ||
parsed_deps is None | ||
or package.name in parsed_deps | ||
or package.source is not Source.SITE_PACKAGE | ||
) | ||
|
||
if underlined: | ||
|
||
def filter_package(package: Package) -> bool: | ||
return package.name not in existing | ||
return package.name not in existing and is_dep(package) | ||
|
||
else: | ||
|
||
def filter_package(package: Package) -> bool: | ||
return package.name not in existing and not package.name.startswith("_") | ||
return ( | ||
package.name not in existing | ||
and not package.name.startswith("_") | ||
and is_dep(package) | ||
) | ||
|
||
return filter(filter_package, packages) | ||
|
||
|
@@ -81,16 +102,15 @@ class AutoImport: | |
""" | ||
|
||
connection: sqlite3.Connection | ||
memory: bool | ||
project: Project | ||
project_package: Package | ||
underlined: bool | ||
prefs: AutoimportPrefs | ||
|
||
def __init__( | ||
self, | ||
project: Project, | ||
observe: bool = True, | ||
underlined: bool = False, | ||
underlined: Optional[bool] = None, | ||
memory: bool = _deprecated_default, | ||
): | ||
"""Construct an AutoImport object. | ||
|
@@ -113,25 +133,28 @@ def __init__( | |
autoimport = AutoImport(..., memory=True) | ||
""" | ||
self.project = project | ||
self.prefs = deepcopy(self.project.prefs.autoimport) | ||
project_package = get_package_tuple(project.root.pathlib, project) | ||
assert project_package is not None | ||
assert project_package.path is not None | ||
if underlined is not None: | ||
self.prefs.underlined = underlined | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we need to copy the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point - I ran a deep copy. Maybe in the future, it should automatically reload on pyproject.toml changes? Would you want me to include that in this PR? |
||
self.project_package = project_package | ||
self.underlined = underlined | ||
self.memory = memory | ||
if memory is _deprecated_default: | ||
self.memory = True | ||
self.prefs.memory = True | ||
warnings.warn( | ||
"The default value for `AutoImport(memory)` argument will " | ||
"change to use an on-disk database by default in the future. " | ||
"If you want to use an in-memory database, you need to pass " | ||
"`AutoImport(memory=True)` explicitly.", | ||
"`AutoImport(memory=True)` explicitly or set it in the config file.", | ||
DeprecationWarning, | ||
) | ||
else: | ||
self.prefs.memory = memory | ||
self.thread_local = local() | ||
self.connection = self.create_database_connection( | ||
project=project, | ||
memory=memory, | ||
memory=self.prefs.memory, | ||
) | ||
self._setup_db() | ||
if observe: | ||
|
@@ -187,7 +210,7 @@ def connection(self): | |
if not hasattr(self.thread_local, "connection"): | ||
self.thread_local.connection = self.create_database_connection( | ||
project=self.project, | ||
memory=self.memory, | ||
memory=self.prefs.memory, | ||
) | ||
return self.thread_local.connection | ||
|
||
|
@@ -389,12 +412,13 @@ def generate_modules_cache( | |
""" | ||
Generate global name cache for external modules listed in `modules`. | ||
|
||
If no modules are provided, it will generate a cache for every module available. | ||
If modules is not specified, uses PEP 621 metadata. | ||
If modules aren't specified and PEP 621 is not present, caches every package | ||
This method searches in your sys.path and configured python folders. | ||
Do not use this for generating your own project's internal names, | ||
use generate_resource_cache for that instead. | ||
""" | ||
underlined = self.underlined if underlined is None else underlined | ||
underlined = self.prefs.underlined if underlined is None else underlined | ||
|
||
packages: List[Package] = ( | ||
self._get_available_packages() | ||
|
@@ -403,12 +427,16 @@ def generate_modules_cache( | |
) | ||
|
||
existing = self._get_packages_from_cache() | ||
packages = list(filter_packages(packages, underlined, existing)) | ||
if not packages: | ||
packages = list( | ||
filter_packages( | ||
packages, underlined, existing, self.project.prefs.dependencies | ||
) | ||
) | ||
if len(packages) == 0: | ||
return | ||
self._add_packages(packages) | ||
job_set = task_handle.create_jobset("Generating autoimport cache", 0) | ||
if single_thread: | ||
if single_thread or not self.prefs.parallel: | ||
for package in packages: | ||
for module in get_files(package, underlined): | ||
job_set.started_job(module.modname) | ||
|
@@ -512,7 +540,7 @@ def update_resource( | |
self, resource: Resource, underlined: bool = False, commit: bool = True | ||
): | ||
"""Update the cache for global names in `resource`.""" | ||
underlined = underlined if underlined else self.underlined | ||
underlined = underlined if underlined else self.prefs.underlined | ||
module = self._resource_to_module(resource, underlined) | ||
self._del_if_exist(module_name=module.modname, commit=False) | ||
for name in get_names(module, self.project_package): | ||
|
@@ -537,7 +565,11 @@ def _del_if_exist(self, module_name, commit: bool = True): | |
|
||
def _get_python_folders(self) -> List[Path]: | ||
def filter_folders(folder: Path) -> bool: | ||
return folder.is_dir() and folder.as_posix() != "/usr/bin" | ||
return ( | ||
folder.is_dir() | ||
and folder.as_posix() != "/usr/bin" | ||
and str(folder) != self.project.address | ||
) | ||
|
||
folders = self.project.get_python_path_folders() | ||
folder_paths = filter(filter_folders, map(Path, folders)) | ||
|
@@ -623,7 +655,7 @@ def _resource_to_module( | |
self, resource: Resource, underlined: bool = False | ||
) -> ModuleFile: | ||
assert self.project_package.path | ||
underlined = underlined if underlined else self.underlined | ||
underlined = underlined if underlined else self.prefs.underlined | ||
resource_path: Path = resource.pathlib | ||
# The project doesn't need its name added to the path, | ||
# since the standard python file layout accounts for that | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
from typing import Iterable | ||
|
||
import pytest | ||
|
||
from rope.contrib.autoimport.sqlite import AutoImport | ||
|
||
|
||
@pytest.fixture | ||
def autoimport(project) -> Iterable[AutoImport]: | ||
autoimport = AutoImport(project, memory=True) | ||
autoimport.generate_modules_cache() | ||
yield autoimport | ||
autoimport.close() | ||
|
||
|
||
@pytest.mark.parametrize("project", ((""),), indirect=True) | ||
def test_blank(project, autoimport): | ||
assert project.prefs.dependencies is None | ||
assert autoimport.search("pytoolconfig") | ||
|
||
|
||
@pytest.mark.parametrize("project", (("[project]\n dependencies=[]"),), indirect=True) | ||
def test_empty(project, autoimport): | ||
assert len(project.prefs.dependencies) == 0 | ||
assert [] == autoimport.search("pytoolconfig") | ||
|
||
|
||
FILE = """ | ||
[project] | ||
dependencies = [ | ||
"pytoolconfig", | ||
"bogus" | ||
] | ||
""" | ||
|
||
|
||
@pytest.mark.parametrize("project", ((FILE),), indirect=True) | ||
def test_not_empty(project, autoimport): | ||
assert len(project.prefs.dependencies) == 2 | ||
assert autoimport.search("pytoolconfig") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd much prefer to keep a flat namespace here for the config keys. The vast majority of users are going to only have a small amount of rope config, creating multiple config sections just for a handful of configuration seems overkill and creates opportunities for errors. Most users wouldn't care/know about rope internals enough to intuitively know that autoimport is a separate package inside rope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, toml allows dotted keys. Maybe we can have our cake and eat it too, by recommending the dotted form in the examples section instead of the separate section: