gemini-code-assist[bot] commented on code in PR #203:
URL: https://github.com/apache/tvm-ffi/pull/203#discussion_r2475037024
##########
python/tvm_ffi/utils/_build_optional_torch_c_dlpack.py:
##########
@@ -665,23 +660,27 @@ def get_torch_include_paths(build_with_cuda: bool) ->
Sequence[str]:
return torch.utils.cpp_extension.include_paths(cuda=build_with_cuda)
-parser = argparse.ArgumentParser()
-parser.add_argument(
- "--build_dir",
- type=str,
- default=str(Path("~/.cache/tvm-ffi/torch_c_dlpack_addon").expanduser()),
- help="Directory to store the built extension library.",
-)
-parser.add_argument(
- "--build_with_cuda",
- action="store_true",
- default=torch.cuda.is_available(),
- help="Build with CUDA support.",
-)
-
-
def main() -> None: # noqa: PLR0912, PLR0915
"""Build the torch c dlpack extension."""
+ # we need to set the following env to avoid tvm_ffi to build the torch
c-dlpack addon during importing
+ os.environ["TVM_FFI_DISABLE_TORCH_C_DLPACK"] = "1"
+ from tvm_ffi.cpp.load_inline import build_ninja # noqa: PLC0415
+ from tvm_ffi.utils.lockfile import FileLock # noqa: PLC0415
+
+ parser = argparse.ArgumentParser()
+ parser.add_argument(
+ "--build_dir",
+ type=str,
+
default=str(Path("~/.cache/tvm-ffi/torch_c_dlpack_addon").expanduser()),
+ help="Directory to store the built extension library.",
+ )
+ parser.add_argument(
+ "--build_with_cuda",
+ action="store_true",
+ default=torch.cuda.is_available(),
+ help="Build with CUDA support.",
+ )
Review Comment:
While moving logic into `main` is a good step for linting, the `main`
function itself is quite long and complex, as indicated by the `# noqa:
PLR0912, PLR0915` suppressions. Since this PR is focused on linting, it would
be a great opportunity to refactor `main` to improve its readability and
maintainability, and potentially remove the `noqa` comments.
Consider breaking down `main` into smaller, more focused helper functions.
For example:
- A function to handle argument parsing (the code you've just moved).
- A function to resolve build configurations (cflags, ldflags,
include_paths), which is a large part of the function.
This would make the main build flow clearer and each part of the logic
easier to understand and test.
--
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]
