Skip to content
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

Add small bit of type hinting #1745

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions src/rez/plugin_managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,26 @@
"""
Manages loading of all types of Rez plugins.
"""
import pkgutil
import os.path
import sys
from types import ModuleType
from typing import Dict, List, Optional, Type
from zipimport import zipimporter

from rez.config import config, expand_system_vars, _load_config_from_filepaths
from rez.utils.formatting import columnise
from rez.utils.schema import dict_to_schema
from rez.utils.data_utils import LazySingleton, cached_property, deep_update
from rez.utils.logging_ import print_debug, print_warning
from rez.vendor.schema.schema import Schema
from rez.exceptions import RezPluginError
from zipimport import zipimporter
import pkgutil
import os.path
import sys


# modified from pkgutil standard library:
# this function is called from the __init__.py files of each plugin type inside
# the 'rezplugins' package.
def extend_path(path, name):
def extend_path(path: List[str], name: str):
"""Extend a package's path.

Intended use is to place the following code in a package's __init__.py:
Expand Down Expand Up @@ -85,23 +89,23 @@ class RezPluginType(object):
'type_name' must correspond with one of the source directories found under
the 'plugins' directory.
"""
type_name = None
type_name: Optional[str] = None

def __init__(self):
if self.type_name is None:
raise TypeError("Subclasses of RezPluginType must provide a "
"'type_name' attribute")
self.pretty_type_name = self.type_name.replace('_', ' ')
self.plugin_classes = {}
self.failed_plugins = {}
self.plugin_modules = {}
self.config_data = {}
self.pretty_type_name: str = self.type_name.replace('_', ' ')
self.plugin_classes: Dict[str, Type[object]] = {}
self.failed_plugins: Dict[str, str] = {}
self.plugin_modules: Dict[str, ModuleType] = {}
self.config_data: Dict[str, Dict] = {}
self.load_plugins()

def __repr__(self):
return '%s(%s)' % (self.__class__.__name__, self.plugin_classes.keys())

def register_plugin(self, plugin_name, plugin_class, plugin_module):
def register_plugin(self, plugin_name: str, plugin_class: Type[object], plugin_module: ModuleType):
# TODO: check plugin_class to ensure it is a sub-class of expected base-class?
# TODO: perhaps have a Plugin base class. This introduces multiple
# inheritance in Shell class though :/
Expand Down Expand Up @@ -222,7 +226,7 @@ def get_plugin_module(self, plugin_name):
% (self.pretty_type_name, plugin_name))

@cached_property
def config_schema(self):
def config_schema(self) -> Schema:
"""Returns the merged configuration data schema for this plugin
type."""
from rez.config import _plugin_config_dict
Expand All @@ -235,7 +239,7 @@ def config_schema(self):
deep_update(d, d_)
return dict_to_schema(d, required=True, modifier=expand_system_vars)

def create_instance(self, plugin, **instance_kwargs):
def create_instance(self, plugin, **instance_kwargs) -> "RezPluginType":

Choose a reason for hiding this comment

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

This doesn't return a RezPluginType instance. It will return something like rezplugins.build_system.cmake.CMakeBuildSystem. I don't think we have an easy way today to get all the possible base classes for each plugin type. It should be possible, but it'll require changes to the plugin system, or some more thoughts to maybe use generics or whatever type magic we'll need to get this to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JeanChristopheMorinPerso Ah right good point. Would it be alright to type hint these just as object instead for now? That at least gives us some kind of information.

"""Create and return an instance of the given plugin."""
return self.get_plugin_class(plugin)(**instance_kwargs)

Expand Down Expand Up @@ -294,7 +298,7 @@ def register_plugin():
'rezplugins' is always found first.
"""
def __init__(self):
self._plugin_types = {}
self._plugin_types: Dict[str, LazySingleton] = {}

@cached_property
def rezplugins_module_paths(self):
Expand Down Expand Up @@ -329,7 +333,7 @@ def rezplugins_module_paths(self):

# -- plugin types

def _get_plugin_type(self, plugin_type):
def _get_plugin_type(self, plugin_type: str) -> RezPluginType:
try:
return self._plugin_types[plugin_type]()
except KeyError:
Expand Down Expand Up @@ -366,7 +370,7 @@ def get_plugin_module(self, plugin_type, plugin_name):
plugin = self._get_plugin_type(plugin_type)
return plugin.get_plugin_module(plugin_name)

def get_plugin_config_data(self, plugin_type):
def get_plugin_config_data(self, plugin_type: str) -> Dict[str, Dict]:
"""Return the merged configuration data for the plugin type."""
plugin = self._get_plugin_type(plugin_type)
return plugin.config_data
Expand Down
Loading