Skip to content

Commit

Permalink
Fix deprecated_submodule conflict with importlib_metadata (quantumlib…
Browse files Browse the repository at this point in the history 
…#4772)

Stop replacing the entire `sys.meta_path` on every use of `deprecated_submodule`
as this may result in multiple-wrap of standard module finders and finders defined by others.
Add a new finder for our renamed module instead, and use a standard import mechanism to
find it under the new module name.
Remove `find_distributions()` and `invalidate_caches()` methods from DeprecatedModuleFinder,
it should be sufficient to only handle these with standard finders and the new module name.
Add test for the conflict with importlib_metadata.distributions() reported in quantumlib#4729.

This fixes quantumlib#4729.
  • Loading branch information
@pavoljuhas
pavoljuhas authored Jan 5, 2022
1 parent e75bcb5 commit 5e1feef
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 143 deletions.
71 changes: 8 additions & 63 deletions cirq/_compat.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,6 @@ class DeprecatedModuleFinder(importlib.abc.MetaPathFinder):
It is meant to be used as a wrapper around existing MetaPathFinder instances.
Args:
finder: The finder to wrap.
new_module_name: The new module's fully qualified name.
old_module_name: The deprecated module's fully qualified name.
deadline: The deprecation deadline.
Expand All @@ -503,35 +502,18 @@ class DeprecatedModuleFinder(importlib.abc.MetaPathFinder):

def __init__(
self,
finder: Any,
new_module_name: str,
old_module_name: str,
deadline: str,
broken_module_exception: Optional[BaseException],
):
"""An aliasing module finder that uses an existing module finder to find a python
"""An aliasing module finder that uses existing module finders to find a python
module spec and intercept the execution of matching modules.
"""
self.finder = finder
self.new_module_name = new_module_name
self.old_module_name = old_module_name
self.deadline = deadline
self.broken_module_exception = broken_module_exception
# to cater for metadata path finders
# https://docs.python.org/3/library/importlib.metadata.html#extending-the-search-algorithm
if hasattr(finder, "find_distributions"):

def find_distributions(context):
return self.finder.find_distributions(context)

self.find_distributions = find_distributions
if hasattr(finder, "invalidate_caches"):

def invalidate_caches() -> None:
return self.finder.invalidate_caches()

# mypy#2427
self.invalidate_caches = invalidate_caches # type: ignore

def find_spec(self, fullname: str, path: Any = None, target: Any = None) -> Any:
"""Finds the specification of a module.
Expand All @@ -547,8 +529,7 @@ def find_spec(self, fullname: str, path: Any = None, target: Any = None) -> Any:
to the wrapped finder.
"""
if fullname != self.old_module_name and not fullname.startswith(self.old_module_name + "."):
# if we are not interested in it, then just pass through to the wrapped finder
return self.finder.find_spec(fullname, path, target)
return None

if self.broken_module_exception is not None:
raise self.broken_module_exception
Expand All @@ -557,33 +538,8 @@ def find_spec(self, fullname: str, path: Any = None, target: Any = None) -> Any:

new_fullname = self.new_module_name + fullname[len(self.old_module_name) :]

# find the corresponding spec in the new structure
if fullname == self.old_module_name:
# this is the first time the deprecated module is being found
# which means that the new parent needs to be found first and under
# the new parent's path, we should be able to find the new name of
# the deprecated module
# this code is heavily inspired by importlib.util.find_spec
parent_name = new_fullname.rpartition('.')[0]
if parent_name:
parent = __import__(parent_name, fromlist=['__path__'])
# note that compared to importlib.util.find_spec we don't handle
# AttributeError here because it is not expected to happen in case
# of a DeprecatedModuleLoader - the new parent should exist and be
# a proper package
parent_path = parent.__path__
else:
parent_path = None
spec = self.finder.find_spec(new_fullname, parent_path, None)
else:
# we are finding a submodule of the parent of the deprecated module,
# which means that the parent was already found, and thus, `path` is
# correctly pointing to the module's parent in the new hierarchy
spec = self.finder.find_spec(
new_fullname,
path=path,
target=target,
)
# use normal import mechanism for the new module specs
spec = importlib.util.find_spec(new_fullname)

# if the spec exists, return the DeprecatedModuleLoader that will do the loading as well
# as set the alias(es) in sys.modules as necessary
Expand Down Expand Up @@ -666,21 +622,10 @@ def deprecated_submodule(
_BrokenModule(new_module_name, broken_module_exception),
)

def wrap(finder: Any) -> Any:
# Sphinx looks for non-wrapped MockFinders
# so we have to check for them and not wrap them
if 'sphinx' in sys.modules:
from sphinx.ext.autodoc.mock import MockFinder

if isinstance(finder, MockFinder):
return finder
if not hasattr(finder, 'find_spec'):
return finder
return DeprecatedModuleFinder(
finder, new_module_name, old_module_name, deadline, broken_module_exception
)

sys.meta_path = [wrap(finder) for finder in sys.meta_path]
finder = DeprecatedModuleFinder(
new_module_name, old_module_name, deadline, broken_module_exception
)
sys.meta_path.append(finder)


def _setup_deprecated_submodule_attribute(
Expand Down
100 changes: 20 additions & 80 deletions cirq/_compat_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
import types
import warnings
from types import ModuleType
from typing import Any, Callable, List, Optional, Sequence, Union
from typing import Any, Callable, Optional
from importlib.machinery import ModuleSpec
from unittest import mock
from importlib.abc import MetaPathFinder


import numpy as np
Expand All @@ -44,7 +43,6 @@
deprecated_class,
deprecated_submodule,
DeprecatedModuleLoader,
DeprecatedModuleFinder,
DeprecatedModuleImportError,
)

Expand Down Expand Up @@ -673,6 +671,25 @@ def _test_metadata_search_path_inner():
assert m.metadata('flynt')


def test_metadata_distributions_after_deprecated_submodule():
subprocess_context(_test_metadata_distributions_after_deprecated_submodule)()


def _test_metadata_distributions_after_deprecated_submodule():
# verify deprecated_submodule does not break importlib_metadata.distributions()
# See https://github.com/quantumlib/Cirq/issues/4729
deprecated_submodule(
new_module_name='cirq.neutral_atoms',
old_parent='cirq',
old_child='swiss_atoms',
deadline="v0.14",
create_attribute=True,
)
m = pytest.importorskip("importlib_metadata")
distlist = list(m.distributions())
assert all(isinstance(d.name, str) for d in distlist)


def test_type_repr_in_new_module():
# to cater for metadata path finders
# https://docs.python.org/3/library/importlib.metadata.html#extending-the-search-algorithm
Expand Down Expand Up @@ -850,18 +867,6 @@ def module_repr(self, module: ModuleType) -> str:
)


def test_invalidate_caches():
called = False

class FakeFinder(importlib.abc.MetaPathFinder):
def invalidate_caches(self) -> None:
nonlocal called
called = True

DeprecatedModuleFinder(FakeFinder(), 'new', 'old', 'v0.1', None).invalidate_caches()
assert called


def test_subprocess_test_failure():
with pytest.raises(Failed, match='ValueError.*this fails'):
subprocess_context(_test_subprocess_test_failure_inner)()
Expand All @@ -882,68 +887,3 @@ def _dir_is_still_valid_inner():

for m in ['fake_a', 'info', 'module_a', 'sys']:
assert m in dir(mod)


class MockModule(ModuleType):
def __init__(self, module_name: str):
ModuleType.__init__(self, module_name)
if '.' in module_name:
package, module = module_name.rsplit('.', 1)
setattr(get_mock_module(package), module, self)

def _initialize_(self, module_code: types.FunctionType):
self.__dict__.update(module_code(self.__name__))


def get_mock_module(module_name: str) -> ModuleType:
if module_name not in sys.modules:
sys.modules[module_name] = MockModule(module_name)
return sys.modules[module_name]


def modulize(module_name: str) -> Callable[[types.FunctionType], Any]:
"""Converts a function into a module:
https://stackoverflow.com/a/45421428/5716192
"""
return get_mock_module(
module_name
)._initialize_ # type: ignore # mypy can't detect the _initialize_ method
# from the MockModule in sys.modules[module_name]


def test_deprecated_module_does_not_wrap_mockfinder():
@modulize('sphinx.ext.autodoc.mock')
def module_code( # pylint: disable=unused-variable # https://github.com/PyCQA/pylint/issues/2842
__name__,
): # pylint: disable=redefined-builtin

# put module code here
class MockFinder(MetaPathFinder):
def __init__(self, modnames: List[str]) -> None:
super().__init__()

def find_spec(
self,
fullname: Optional[str] = None,
path: Optional[Sequence[Union[bytes, str]]] = None,
target: Optional[ModuleType] = None,
) -> None:
pass

# the function must return locals()
return locals()

from sphinx.ext.autodoc.mock import MockFinder

fake_mockfinder = MockFinder([])
sys.meta_path.insert(0, fake_mockfinder)
deprecated_submodule(
new_module_name='sphinx_1',
old_parent='sphinx_2',
old_child='old_ch',
deadline='v1.2',
create_attribute=False,
)
assert fake_mockfinder in sys.meta_path
# Cleanup sys.metapath after test
sys.meta_path.remove(fake_mockfinder)

0 comments on commit 5e1feef

Please sign in to comment.