Skip to content

Conversation

benrobby
Copy link

@benrobby benrobby commented Sep 5, 2025

What changes were proposed in this pull request?

Why are the changes needed?

  • fix Build / Python-only (master, Minimum dependencies of PySpark)

Does this PR introduce any user-facing change?

No

How was this patch tested?

ran tests locally with numpy 1.22.4

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

No

return tuple(convert_to_numpy_printable(elem) for elem in x)
elif isinstance(x, dict):
return {k: convert_to_numpy_printable(v) for k, v in x.items()}
elif hasattr(x, "dtype") and hasattr(x, "item"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will capture numpy array. Are we targeting at scalars only isinstance(x, np.generic)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, added an isinstance(x, np.generic)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np.generic is the base class for all numpy scalar types, why do we still need to check hasattr(x, "dtype") and hasattr(x, "item")?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also removed them now

@xinrong-meng xinrong-meng self-requested a review September 5, 2025 17:35
@benrobby benrobby changed the title [SPARK-53355][PYTHON] fix numpy 1.x repr in type tests [SPARK-53355][PYTHON][SQL] fix numpy 1.x repr in type tests Sep 8, 2025
@benrobby
Copy link
Author

benrobby commented Sep 8, 2025

@xinrong-meng this is ready from my side. Is there a way to trigger the master tests with the different envs on this PR? I have run this locally, but it would be nice to confirm additionally in CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants