gemini-code-assist[bot] commented on code in PR #333:
URL: https://github.com/apache/tvm-ffi/pull/333#discussion_r2612131231


##########
tests/lint/check_version.py:
##########
@@ -157,10 +157,46 @@ def _check_rust(version_info: dict) -> list[str]:
     return errors
 
 
+def _check_torch_c_dlpack_ext(version_info: dict) -> list[str]:
+    errors: list[str] = []
+    path = Path("addons") / "torch_c_dlpack_ext" / "pyproject.toml"
+
+    if not path.exists():
+        errors.append(f"[torch_c_dlpack_ext] Missing expected pyproject.toml 
at {path}")
+        return errors
+
+    data = tomli.loads(path.read_text(encoding="utf-8"))

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The call to `tomli.loads` can raise a `tomli.TOMLDecodeError` if the 
`pyproject.toml` file is malformed. This would crash the script. It's better to 
handle this exception gracefully and report it as a linting error to improve 
the script's robustness.
   
   ```suggestion
       try:
           data = tomli.loads(path.read_text(encoding="utf-8"))
       except tomli.TOMLDecodeError as e:
           errors.append(f"[torch_c_dlpack_ext] {path}: Failed to parse TOML: 
{e}")
           return errors
   ```



##########
tests/lint/check_version.py:
##########
@@ -157,10 +157,46 @@ def _check_rust(version_info: dict) -> list[str]:
     return errors
 
 
+def _check_torch_c_dlpack_ext(version_info: dict) -> list[str]:
+    errors: list[str] = []
+    path = Path("addons") / "torch_c_dlpack_ext" / "pyproject.toml"
+
+    if not path.exists():
+        errors.append(f"[torch_c_dlpack_ext] Missing expected pyproject.toml 
at {path}")
+        return errors
+
+    data = tomli.loads(path.read_text(encoding="utf-8"))
+    version = data.get("project", {}).get("version")
+
+    if version is None:
+        errors.append(f"[torch_c_dlpack_ext] {path}: Missing 
[project].version")
+        return errors
+
+    base = version_info["base_version"]
+    allowed: set[str] = {base}
+    pre = _map_pep440_pre_to_semver(version_info.get("pre"))
+    if pre:
+        allowed.add(f"{base}-{pre}")
+    allowed = sorted(allowed)

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   This block of code for determining allowed versions is duplicated from the 
`_check_rust` function (lines 146-151). To improve maintainability and avoid 
code duplication, consider extracting this logic into a separate helper 
function.
   
   For example, you could create a function `_get_allowed_versions`:
   ```python
   def _get_allowed_versions(version_info: dict) -> list[str]:
       """Computes the allowed version strings based on the project version."""
       base = version_info["base_version"]
       allowed: set[str] = {base}
       pre = _map_pep440_pre_to_semver(version_info.get("pre"))
       if pre:
           allowed.add(f"{base}-{pre}")
       return sorted(allowed)
   ```
   Then, you can replace this block with `allowed = 
_get_allowed_versions(version_info)` here and in `_check_rust`.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to