Skip to content

Commit

Permalink
[SPARK-50824][PYTHON] Avoid importing optional Python packages for ch…
Browse files Browse the repository at this point in the history
…ecking

### What changes were proposed in this pull request?

This PR proposes to avoid importing optional Python packages for checking, by using `importlib.util.find_spec` instead of actually loading/importing the package.

### Why are the changes needed?

a409199 changed to import optional dependencies in main code. After that, technically f223b8d broke the Python Spark Core tests, (because now we will import `pyspark.testing`, and it will import optional dependencies) but it did not run the tests.

By importing `deepspeed`, via logger, it can show stdout (https://github.com/microsoft/DeepSpeed/blob/master/accelerator/real_accelerator.py#L182). This broke the test in `pyspark.conf`. After that, the real test failure was found when core change was triggered at 6f3b778. In the PR, build passed because it was before f223b8d was merged.

### Does this PR introduce _any_ user-facing change?

Technically yes. There might be some side effects by importing optional dependencies, and this PR avoid them.

### How was this patch tested?

CI in this PR should verify it.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #49500 from HyukjinKwon/SPARK-50824.

Authored-by: Hyukjin Kwon <gurwls223@apache.org>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
  • Loading branch information
HyukjinKwon committed Jan 15, 2025
1 parent 0593ac6 commit 7e1248a
Showing 1 changed file with 2 additions and 6 deletions.
8 changes: 2 additions & 6 deletions python/pyspark/testing/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,9 @@


def have_package(name: str) -> bool:
try:
import importlib
import importlib

importlib.import_module(name)
return True
except Exception:
return False
return importlib.util.find_spec(name) is not None


have_numpy = have_package("numpy")
Expand Down

0 comments on commit 7e1248a

Please sign in to comment.