Skip to content

[mlir:python] Fail immediately if importing an initializer module raises ImportError #74595

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

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

hawkinsp
Copy link
Contributor

@hawkinsp hawkinsp commented Dec 6, 2023

If ImportError is raised, _site_initialize will fail because 'm' will be an unbound local. Just return False in this case and fail fast.

@stellaraccident @makslevental

ImportError.

If ImportError is raised, _site_initialize will fail because 'm' will
be an unbound local. Just return False in this case and fail fast.
@llvmbot llvmbot added mlir:python MLIR Python bindings mlir labels Dec 6, 2023
@hawkinsp hawkinsp changed the title [mlir:python] Fail immediately if importing an initializer module raises [mlir:python] Fail immediately if importing an initializer module raises ImportError Dec 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-mlir

Author: Peter Hawkins (hawkinsp)

Changes

If ImportError is raised, _site_initialize will fail because 'm' will be an unbound local. Just return False in this case and fail fast.

@stellaraccident @makslevental


Full diff: https://github.com/llvm/llvm-project/pull/74595.diff

1 Files Affected:

  • (modified) mlir/python/mlir/_mlir_libs/init.py (+1)
diff --git a/mlir/python/mlir/_mlir_libs/__init__.py b/mlir/python/mlir/_mlir_libs/__init__.py
index 32f46d24cc739..98dbbc6adf9ce 100644
--- a/mlir/python/mlir/_mlir_libs/__init__.py
+++ b/mlir/python/mlir/_mlir_libs/__init__.py
@@ -94,6 +94,7 @@ def process_initializer_module(module_name):
                 "encountered otherwise and the MLIR Python API may not function."
             )
             logger.warning(message, exc_info=True)
+            return False
 
         logger.debug("Initializing MLIR with module: %s", module_name)
         if hasattr(m, "register_dialects"):

Copy link
Contributor

@stellaraccident stellaraccident left a comment

Choose a reason for hiding this comment

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

Thanks

@makslevental
Copy link
Contributor

Thanks @hawkinsp - do you need me to merge?

@hawkinsp
Copy link
Contributor Author

hawkinsp commented Dec 6, 2023

@makslevental yes please! I have no LLVM committer privileges.

@makslevental makslevental merged commit 45e7b41 into llvm:main Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:python MLIR Python bindings mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants