From e46c6622b27a7716533b5f81a5022034dcb0b295 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 5 Feb 2024 13:04:38 +0100 Subject: [PATCH 1/2] [PyROOT] Drop support for `from ROOT import *` Wild card imports like `from ROOT import *` stopped working with Python 3.11 given some changes in the CPython API. For that reason, upstream `cppyy` also dropped support for lazy lookups, which was the feature that enabled wildcard imports: https://github.com/wlav/CPyCppyy/commit/64fd89050a66bf8cb119f236cadd365efa07b005#diff-6160c0eb004dabeedaeb58d804a7ecd3e563d9379e9e7af39623fd38d0bc6a37R352 This commit suggests to document `from ROOT import *` officially as unsupported and remove the corresponding code path in the ROOT facade. Closes #7669. --- README/ReleaseNotes/v632/index.md | 1 + .../pythonizations/python/ROOT/_facade.py | 23 +------------------ 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/README/ReleaseNotes/v632/index.md b/README/ReleaseNotes/v632/index.md index f6de3c0d75ff5..8a44ca5538c61 100644 --- a/README/ReleaseNotes/v632/index.md +++ b/README/ReleaseNotes/v632/index.md @@ -44,6 +44,7 @@ The following people have contributed to this new version: - Some redundant **RooDataSet** constructors are deprecated and will be removed in ROOT 6.34. Please use the RooDataSet constructors that take RooFit command arguments instead - ROOT does not longer support Python 2. The minimum required Python version to build ROOT is 3.8. +- Support for wildcard imports like `from ROOT import *` is dropped from PyROOT ## Core Libraries diff --git a/bindings/pyroot/pythonizations/python/ROOT/_facade.py b/bindings/pyroot/pythonizations/python/ROOT/_facade.py index d90094832eb1b..8440468b7e52f 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_facade.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_facade.py @@ -145,22 +145,6 @@ def AddressOf(self, obj): # Create a buffer (LowLevelView) from address return cppyy.ll.cast[out_type](addr) - def _handle_import_all(self): - # Called if "from ROOT import *" is executed in the app. - # Customises lookup in Python's main module to also - # check in C++'s global namespace - - # Get caller module (jump over the facade frames) - num_frame = 2 - frame = sys._getframe(num_frame).f_globals["__name__"] - while frame == "ROOT._facade": - num_frame += 1 - frame = sys._getframe(num_frame).f_globals["__name__"] - caller = sys.modules[frame] - - # Install the hook - cppyy_backend._set_cpp_lazy_lookup(caller.__dict__) - def _fallback_getattr(self, name): # Try: # - in the global namespace @@ -170,13 +154,8 @@ def _fallback_getattr(self, name): # The first two attempts allow to lookup # e.g. ROOT.ROOT.Math as ROOT.Math - if name == "__all__": - self._handle_import_all() - # Make the attributes of the facade be injected in the - # caller module - raise AttributeError() # Note that hasattr caches the lookup for getattr - elif hasattr(gbl_namespace, name): + if hasattr(gbl_namespace, name): return getattr(gbl_namespace, name) elif hasattr(gbl_namespace.ROOT, name): return getattr(gbl_namespace.ROOT, name) From f612fda647e872e8ce9c8d4c0449b7e832854718 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 5 Feb 2024 14:36:18 +0100 Subject: [PATCH 2/2] [PyROOT] Actively prevent wildcard imports from ROOT and its submodules With this commit, the user will get an `ImportError` when trying to do `from ROOT import *` or for example `from ROOT.RooFit import *`. Before this commit, such commands were succeeding but without actually importing anything, which might me confusing to the user because the expect that everything from ROOT gets imported. --- .../pythonizations/python/ROOT/__init__.py | 27 ++++++++++++++++++- .../pythonizations/python/ROOT/_facade.py | 3 +-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/bindings/pyroot/pythonizations/python/ROOT/__init__.py b/bindings/pyroot/pythonizations/python/ROOT/__init__.py index ef89379f4178a..839b33c03c54e 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/__init__.py +++ b/bindings/pyroot/pythonizations/python/ROOT/__init__.py @@ -55,6 +55,26 @@ _is_ipython = hasattr(builtins, "__IPYTHON__") + +class _PoisonedDunderAll: + """ + Dummy class used to trigger an ImportError on wildcard imports if the + `__all__` attribute of a module is an instance of this class. + """ + + def __getitem__(self, _): + import textwrap + + message = """ + Wildcard import e.g. `from module import *` is bad practice, so it is disallowed in ROOT. Please import explicitly. + """ + raise ImportError(textwrap.dedent(message)) + + +# Prevent `from ROOT import *` by setting the __all__ attribute to something +# that will raise an ImportError on item retrieval. +__all__ = _PoisonedDunderAll() + # Configure ROOT facade module import sys from ._facade import ROOTFacade @@ -127,7 +147,12 @@ def is_package(self, fullname: str) -> bool: return _lookup_root_module(fullname) is not None def create_module(self, spec: ModuleSpec): - return _lookup_root_module(spec.name) + out = _lookup_root_module(spec.name) + # Prevent wildcard import for the submodule by setting the __all__ + # attribute to something that will raise an ImportError on item + # retrieval. + out.__all__ = _PoisonedDunderAll() + return out def exec_module(self, module): pass diff --git a/bindings/pyroot/pythonizations/python/ROOT/_facade.py b/bindings/pyroot/pythonizations/python/ROOT/_facade.py index 8440468b7e52f..b086204aaba70 100644 --- a/bindings/pyroot/pythonizations/python/ROOT/_facade.py +++ b/bindings/pyroot/pythonizations/python/ROOT/_facade.py @@ -82,9 +82,8 @@ def __init__(self, module, is_ipython): types.ModuleType.__init__(self, module.__name__) self.module = module - # Importing all will be customised later - self.module.__all__ = [] + self.__all__ = module.__all__ self.__name__ = module.__name__ self.__file__ = module.__file__ self.__cached__ = module.__cached__