Conversation
There was a problem hiding this comment.
Code Review
This pull request aims to fix the documentation build by removing ray.data._internal.arrow_ops from the list of mocked modules. This change seems plausible, but it might be incomplete. I've pointed out that a dependency, ray.air.util.tensor_extensions, is still mocked, which could cause new import errors. I recommend also removing ray.air.util.tensor_extensions from the mocked list to prevent this. For future pull requests, providing a more descriptive title and a body explaining the problem and the fix would be very helpful for reviewers.
| "ray.core.generated", | ||
| "ray.serve.generated", | ||
| "ray.air.util.tensor_extensions", | ||
| "ray.data._internal.arrow_ops", |
There was a problem hiding this comment.
While removing ray.data._internal.arrow_ops from the mocked imports is a good step, this change might be incomplete and could lead to further import errors during the documentation build.
The module ray.data._internal.arrow_ops.transform_pyarrow (which will now be imported) has a dependency on ray.air.util.tensor_extensions.arrow. However, ray.air.util.tensor_extensions is still listed in autodoc_mock_imports on the line above. Mocking the parent package ray.air.util.tensor_extensions will likely cause the import of its submodule arrow to fail.
To fully resolve the issue, you should consider also removing ray.air.util.tensor_extensions from autodoc_mock_imports. Its own dependencies like numpy and pyarrow are already mocked, so it should be safe to un-mock it.
49f450d to
df8709d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
dstrodtman
left a comment
There was a problem hiding this comment.
I believe I'm currently seeing the related errors here. Haven't tested the fix, but stamping to unblock
Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
df8709d to
5ebca78
Compare
No description provided.