sjyangkevin commented on code in PR #53690:
URL: https://github.com/apache/airflow/pull/53690#discussion_r2238313322
##########
airflow-core/src/airflow/serialization/serializers/numpy.py:
##########
@@ -69,7 +70,7 @@ def serialize(o: object) -> tuple[U, str, int, bool]:
):
return int(o), name, __version__, True
- if isinstance(o, np.bool_):
+ if hasattr(np, "bool") and isinstance(o, np.bool) or isinstance(o,
np.bool_):
Review Comment:
interesting implementation. I think here we are leveraging short-circuit to
assign `np.bool_` to `np_bool` and then avoid the checking to `np.bool` when
that is not needed. I think it works in both NumPy v1 and v2. Since in v2,
`np.bool_` is an alias of `np.bool`. I think all these are feasible options.
I think the main concern is not directly related to this check. As shown
below, the serializers is a list for checking serializable objects based on the
qualified name. It is why both `np.bool` and `np.bool_` come into the place. In
NumPy < 2.0, objects created with `np.bool_(True|False)` resolved to `np.bool_`
qualified name, but in NumPy >= 2.0, object's qualified name is resolved to
`np.bool`. A straight-forward implementation to resolve this compatibility
issue is to include both `np.bool` and `np.bool_` into the list.
https://github.com/apache/airflow/blob/584dc0b2c424ca76e4f4ccbc589487dd8a8fa57b/airflow-core/src/airflow/serialization/serializers/numpy.py#L25-L40
--
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]
