carloea2 commented on code in PR #8597:
URL: https://github.com/apache/texera/pull/8597#discussion_r4254316874
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/typecasting/TypeCastingOpDesc.scala:
##########
@@ -117,23 +117,27 @@ class TypeCastingOpDesc extends MapOpDesc with
StandaloneCodeGenerator {
// declared type still says so: the values reach the helper as
floats.
val wrap =
!declared.get(unit.attribute).contains(AttributeType.DOUBLE)
val wrapArg = if (wrap) "True" else "False"
- s"""out1df[$colLit].apply(lambda x: pd.NA if pd.isna(x) else
_texera_cast_int32(x, $wrapArg)).astype("Int64")"""
+ // The column's mask decides the holes, so a stored NaN is cast like
+ // any other double; see `_texera_cast_cells`.
+ s"""_texera_cast_cells(out1df[$colLit], lambda x:
_texera_cast_int32(x, $wrapArg), pd.NA).astype("Int64")"""
case AttributeType.LONG =>
- s"""out1df[$colLit].apply(lambda x: pd.NA if pd.isna(x) else
_texera_cast_integral(x)).astype("Int64")"""
+ s"""_texera_cast_cells(out1df[$colLit], _texera_cast_integral,
pd.NA).astype("Int64")"""
case AttributeType.DOUBLE =>
// NaN rather than pd.NA: float64 is how a double column is held
here,
// and it carries its hole as NaN. pd.NA would not survive the
astype.
s"""out1df[$colLit].apply(lambda x: float("nan") if pd.isna(x) else
_texera_cast_double(x)).astype("float64")"""
case AttributeType.BOOLEAN =>
// Nullable "boolean" for the same reason, and because
`.astype(bool)`
// reads NaN as True: NaN is a non-zero float.
- s"""out1df[$colLit].apply(lambda x: pd.NA if pd.isna(x) else
_texera_cast_boolean(x)).astype("boolean")"""
+ s"""_texera_cast_cells(out1df[$colLit], _texera_cast_boolean,
pd.NA).astype("boolean")"""
case AttributeType.TIMESTAMP =>
- // A number is an instant in milliseconds and needs its own reading;
- // see the helper. Text keeps the parser it already had.
+ // A number is an instant in milliseconds and needs its own reading.
+ // Text is read a cell at a time, and over the years the engine
+ // reaches rather than the ones pandas parses into by default; see
+ // the helpers.
if (declared.get(unit.attribute).contains(AttributeType.LONG))
s"""_texera_epoch_millis_to_timestamp(out1df[$colLit])"""
Review Comment:
A Windows case remains in this path: nullable Int64 epoch milliseconds for
1600-06-01 cause OSError from dateutil.tzlocal during tz_localize(None), while
native parseField returns the timestamp. Year 2024, year 2500 and null controls
pass. This is separate from the repaired text path. Could the LONG path avoid
the Windows localtime limit and cover this case?
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/typecasting/TypeCastingOpDesc.scala:
##########
@@ -117,23 +117,27 @@ class TypeCastingOpDesc extends MapOpDesc with
StandaloneCodeGenerator {
// declared type still says so: the values reach the helper as
floats.
val wrap =
!declared.get(unit.attribute).contains(AttributeType.DOUBLE)
val wrapArg = if (wrap) "True" else "False"
- s"""out1df[$colLit].apply(lambda x: pd.NA if pd.isna(x) else
_texera_cast_int32(x, $wrapArg)).astype("Int64")"""
+ // The column's mask decides the holes, so a stored NaN is cast like
+ // any other double; see `_texera_cast_cells`.
+ s"""_texera_cast_cells(out1df[$colLit], lambda x:
_texera_cast_int32(x, $wrapArg), pd.NA).astype("Int64")"""
case AttributeType.LONG =>
- s"""out1df[$colLit].apply(lambda x: pd.NA if pd.isna(x) else
_texera_cast_integral(x)).astype("Int64")"""
+ s"""_texera_cast_cells(out1df[$colLit], _texera_cast_integral,
pd.NA).astype("Int64")"""
case AttributeType.DOUBLE =>
// NaN rather than pd.NA: float64 is how a double column is held
here,
// and it carries its hole as NaN. pd.NA would not survive the
astype.
s"""out1df[$colLit].apply(lambda x: float("nan") if pd.isna(x) else
_texera_cast_double(x)).astype("float64")"""
Review Comment:
This DOUBLE branch still discards the null mask. I ran two generated casts
on ["NaN", null, "1.25", "Infinity"], first to DOUBLE and then STRING. Native
parseField returns ["NaN", null, "1.25", "Infinity"]; the export returns [null,
null, "1.25", "Infinity"]. The finite and null control passes. Please retain
NaN separately from missing cells here too and add the chained case.
--
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]
