harshith1118 commented on code in PR #1402:
URL: https://github.com/apache/hamilton/pull/1402#discussion_r2404103667
##########
hamilton/cli/logic.py:
##########
@@ -337,3 +338,43 @@ def _read_py_context(file_path: Path) -> dict:
context[k] = getattr(module, k, None)
return context
+
+
+def _read_toml_context(file_path: Path) -> dict:
+ """Read context from a TOML file. For pyproject.toml, looks for Hamilton
configuration in [tool.hamilton] section."""
+ try:
+ import tomli # Using tomli for compatibility with older Python
versions
+ except ImportError:
+ # Provide a helpful error message if tomli is not available
+ raise ImportError(
+ "tomli is required to read TOML files. "
+ "Install it with `pip install tomli` or `pip install
sf-hamilton[cli]` which includes TOML support."
Review Comment:
You're correct to point this out. Looking at the error message in
`hamilton/cli/logic.py`, it
indicates that `tomli` should be included with the CLI optional
dependency (`sf-hamilton[cli]`),
but it appears that `tomli` may not be properly listed in the optional
dependencies configuration
(likely in `pyproject.toml`).
The error message suggests that `tomli` should be installed as part of
`sf-hamilton[cli]`, but if
it's not configured in the package dependencies, this is misleading. We
should either:
1. Add `tomli` to the optional dependencies in the package
configuration, OR
2. Update the error message to accurately reflect the installation
requirement
Looking at the intended behavior from the error message, it seems like
`tomli` was meant to be
included as part of the CLI optional dependencies, so the package
configuration likely needs to
be updated to include it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
