-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[MLIR][Python] remove PyYAML as a dep #169145
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
[MLIR][Python] remove PyYAML as a dep #169145
Conversation
0b42c3d to
18d16bb
Compare
|
@llvm/pr-subscribers-infrastructure @llvm/pr-subscribers-mlir Author: Maksim Levental (makslevental) ChangesPyYAML is not an actual use-time/runtime dependency of our bindings. It is only used during build time - during
This PR does the minimal refactor to remove the need during actual run/use time.
It turns out, even without explicitly installing, PyYAML is somehow installed (probably a transitive dep of one of the other deps in Note, I would prefer to also split Full diff: https://github.com/llvm/llvm-project/pull/169145.diff 1 Files Affected:
diff --git a/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py b/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py
index 1672656b3a1f8..f3ad92d782eff 100644
--- a/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py
+++ b/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py
@@ -4,23 +4,30 @@
"""YAML serialization is routed through here to centralize common logic."""
import sys
+import warnings
+
+
+def multiline_str_representer(dumper, data):
+ if len(data.splitlines()) > 1:
+ return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="|")
+ else:
+ return dumper.represent_scalar("tag:yaml.org,2002:str", data)
+
try:
- import yaml
+ from yaml import YAMLObject, add_representer
+
+ add_representer(str, multiline_str_representer)
except ModuleNotFoundError as e:
- raise ModuleNotFoundError(
- f"This tool requires PyYAML but it was not installed. "
- f"Recommend: {sys.executable} -m pip install PyYAML"
- ) from e
+ warnings.warn(
+ "PyYAML is not installed; you will not be able to generate linalg yaml definitions."
+ )
-__all__ = [
- "yaml_dump",
- "yaml_dump_all",
- "YAMLObject",
-]
+ class YAMLObject:
+ pass
-class YAMLObject(yaml.YAMLObject):
+class YAMLObject(YAMLObject):
@classmethod
def to_yaml(cls, dumper, self):
"""Default to a custom dictionary mapping."""
@@ -33,21 +40,34 @@ def as_linalg_yaml(self):
return yaml_dump(self)
-def multiline_str_representer(dumper, data):
- if len(data.splitlines()) > 1:
- return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="|")
- else:
- return dumper.represent_scalar("tag:yaml.org,2002:str", data)
+def yaml_dump(data, sort_keys=False, **kwargs):
+ try:
+ import yaml
+ return yaml.dump(data, sort_keys=sort_keys, **kwargs)
+ except ModuleNotFoundError as e:
+ raise ModuleNotFoundError(
+ f"This tool requires PyYAML but it was not installed. "
+ f"Recommend: {sys.executable} -m pip install PyYAML"
+ ) from e
-yaml.add_representer(str, multiline_str_representer)
+def yaml_dump_all(data, sort_keys=False, explicit_start=True, **kwargs):
+ try:
+ import yaml
-def yaml_dump(data, sort_keys=False, **kwargs):
- return yaml.dump(data, sort_keys=sort_keys, **kwargs)
+ return yaml.dump_all(
+ data, sort_keys=sort_keys, explicit_start=explicit_start, **kwargs
+ )
+ except ModuleNotFoundError as e:
+ raise ModuleNotFoundError(
+ f"This tool requires PyYAML but it was not installed. "
+ f"Recommend: {sys.executable} -m pip install PyYAML"
+ ) from e
-def yaml_dump_all(data, sort_keys=False, explicit_start=True, **kwargs):
- return yaml.dump_all(
- data, sort_keys=sort_keys, explicit_start=explicit_start, **kwargs
- )
+__all__ = [
+ "yaml_dump",
+ "yaml_dump_all",
+ "YAMLObject",
+]
|
|
@llvm/pr-subscribers-mlir-linalg Author: Maksim Levental (makslevental) ChangesPyYAML is not an actual use-time/runtime dependency of our bindings. It is only used during build time - during
This PR does the minimal refactor to remove the need during actual run/use time.
It turns out, even without explicitly installing, PyYAML is somehow installed (probably a transitive dep of one of the other deps in Note, I would prefer to also split Full diff: https://github.com/llvm/llvm-project/pull/169145.diff 1 Files Affected:
diff --git a/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py b/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py
index 1672656b3a1f8..f3ad92d782eff 100644
--- a/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py
+++ b/mlir/python/mlir/dialects/linalg/opdsl/lang/yaml_helper.py
@@ -4,23 +4,30 @@
"""YAML serialization is routed through here to centralize common logic."""
import sys
+import warnings
+
+
+def multiline_str_representer(dumper, data):
+ if len(data.splitlines()) > 1:
+ return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="|")
+ else:
+ return dumper.represent_scalar("tag:yaml.org,2002:str", data)
+
try:
- import yaml
+ from yaml import YAMLObject, add_representer
+
+ add_representer(str, multiline_str_representer)
except ModuleNotFoundError as e:
- raise ModuleNotFoundError(
- f"This tool requires PyYAML but it was not installed. "
- f"Recommend: {sys.executable} -m pip install PyYAML"
- ) from e
+ warnings.warn(
+ "PyYAML is not installed; you will not be able to generate linalg yaml definitions."
+ )
-__all__ = [
- "yaml_dump",
- "yaml_dump_all",
- "YAMLObject",
-]
+ class YAMLObject:
+ pass
-class YAMLObject(yaml.YAMLObject):
+class YAMLObject(YAMLObject):
@classmethod
def to_yaml(cls, dumper, self):
"""Default to a custom dictionary mapping."""
@@ -33,21 +40,34 @@ def as_linalg_yaml(self):
return yaml_dump(self)
-def multiline_str_representer(dumper, data):
- if len(data.splitlines()) > 1:
- return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="|")
- else:
- return dumper.represent_scalar("tag:yaml.org,2002:str", data)
+def yaml_dump(data, sort_keys=False, **kwargs):
+ try:
+ import yaml
+ return yaml.dump(data, sort_keys=sort_keys, **kwargs)
+ except ModuleNotFoundError as e:
+ raise ModuleNotFoundError(
+ f"This tool requires PyYAML but it was not installed. "
+ f"Recommend: {sys.executable} -m pip install PyYAML"
+ ) from e
-yaml.add_representer(str, multiline_str_representer)
+def yaml_dump_all(data, sort_keys=False, explicit_start=True, **kwargs):
+ try:
+ import yaml
-def yaml_dump(data, sort_keys=False, **kwargs):
- return yaml.dump(data, sort_keys=sort_keys, **kwargs)
+ return yaml.dump_all(
+ data, sort_keys=sort_keys, explicit_start=explicit_start, **kwargs
+ )
+ except ModuleNotFoundError as e:
+ raise ModuleNotFoundError(
+ f"This tool requires PyYAML but it was not installed. "
+ f"Recommend: {sys.executable} -m pip install PyYAML"
+ ) from e
-def yaml_dump_all(data, sort_keys=False, explicit_start=True, **kwargs):
- return yaml.dump_all(
- data, sort_keys=sort_keys, explicit_start=explicit_start, **kwargs
- )
+__all__ = [
+ "yaml_dump",
+ "yaml_dump_all",
+ "YAMLObject",
+]
|
357ed29 to
71141f0
Compare
71141f0 to
e917f96
Compare
ashermancinelli
left a comment
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.
Much appreciated!
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/22185 Here is the relevant piece of the build log for the reference |
PyYAML is not an actual use-time/runtime dependency of our bindings. It is necessary only if someone wants to regenerate `LinalgNamedStructuredOps.yaml`: https://github.com/llvm/llvm-project/blob/93097b2d47c87bf5eee0a2612d961c7a01831eab/mlir/tools/mlir-linalg-ods-gen/update_core_linalg_named_ops.sh.in#L29 This PR does the minimal refactor to remove the need during actual run/use time.
PyYAML is not an actual use-time/runtime dependency of our bindings. It is necessary only if someone wants to regenerate `LinalgNamedStructuredOps.yaml`: https://github.com/llvm/llvm-project/blob/93097b2d47c87bf5eee0a2612d961c7a01831eab/mlir/tools/mlir-linalg-ods-gen/update_core_linalg_named_ops.sh.in#L29 This PR does the minimal refactor to remove the need during actual run/use time.
PyYAML is not an actual use-time/runtime dependency of our bindings. It is necessary only if someone wants to regenerate
LinalgNamedStructuredOps.yaml:llvm-project/mlir/tools/mlir-linalg-ods-gen/update_core_linalg_named_ops.sh.in
Line 29 in 93097b2
This PR does the minimal refactor to remove the need during actual run/use time.
Note, since we do actually have tests for
linalg.opdslin-tree, the way I'm testing this PR is by temporarily removing PyYAML from.ci/all_requirements.txt(i.e., not installing during pre-commit) and showing that all other tests pass. Note,linalg.opdsltests are excluded ifPyYAMLis not installed:llvm-project/mlir/test/python/dialects/linalg/opdsl/lit.local.cfg
Lines 6 to 9 in f9008e6
After the PR is reviewed/approved, I will restore
.ci/all_requirements.txt.Note, I would prefer to also split
mlir/python/requirements.txtinto a pair of requirements files (one which contains only the runtime deps and one which contains both the runtime and build deps) but probably that would break all of the builders so I'm leaving it as is.