Skip to content

Fix encoded_missing_value being ignored and all-missing datetime columns crashing#76

Open
devYRPauli wants to merge 2 commits into
google-research:mainfrom
devYRPauli:fix/encoder-missing-value-and-nat-datetime
Open

Fix encoded_missing_value being ignored and all-missing datetime columns crashing#76
devYRPauli wants to merge 2 commits into
google-research:mainfrom
devYRPauli:fix/encoder-missing-value-and-nat-datetime

Conversation

@devYRPauli

Copy link
Copy Markdown
Contributor

Two independent edge case fixes in tabfm/src/classifier_and_regressor.py, one per commit.

1. CategoricalOrdinalEncoder ignores encoded_missing_value

encoded_missing_value is accepted in __init__ and documented as "Encoded value used for NaN / missing values", but transform() never reads it. Missing values and unseen categories are both filled with unknown_value, so the parameter has no effect.

enc = CategoricalOrdinalEncoder(unknown_value=-1, encoded_missing_value=-99)
enc.fit(np.array([["known"], ["other"]], dtype=object))
enc.transform(np.array([[np.nan], ["unseen"], ["known"]], dtype=object))
# before: [-1., -1., 0.]
# after:  [-99., -1., 0.]

The fix records the original NaN positions before mapping and fills those with encoded_missing_value, leaving unknown_value for values genuinely absent from the category map. Both parameters default to -1, so behaviour is unchanged unless a caller sets them differently.

2. DatetimeTransformer crashes on an all-missing datetime column

fit() stores series.mean() as the fill value. For a column with no valid timestamps that mean is NaT, so the later fillna is a no-op and the subsequent .astype(np.int64) on the extracted parts raises:

DatetimeTransformer().fit_transform(pd.DataFrame({"ts": pd.to_datetime([None, None, None])}))
# before: pandas.errors.IntCastingNaNError: Cannot convert non-finite values (NA or inf) to integer
# after:  completes, shape (3, 5), all values finite

The fix falls back to the epoch when the computed mean is NaT. Columns with at least one valid timestamp are unaffected.

Tests

Added CategoricalOrdinalEncoderTest and DatetimeTransformerTest to classifier_and_regressor_test.py. Both were confirmed to fail against the unpatched source (-1.0 vs -99 for the first, IntCastingNaNError for the second) and pass with the change.

Full existing suite after the change: 48 tests run, 20 passed, 28 skipped because JAX is not installed in my environment. Those 28 remain unverified locally.

These changes do not touch any line modified by my open PRs #59 or #69.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant