Repository navigation
BUG: to_json not warning about deprecated epoch date_format for datetime index/columns (#65868) - #65869
Conversation
f1b67c2 to
45c681f
Compare
|
Heads-up on CI: extending the warning to the index/columns (as intended) correctly surfaces the deprecated default These now emit I'm happy to update all the affected tests the same way in this PR. Before I push a large test-only diff: do you prefer wrapping them in |
…ime index/columns (GH#65868) The default date_format="epoch" deprecation warning in to_json only checked the column-value dtypes, so a datetime-like index or set of column labels produced epoch integers without warning, even though those axes are also epoch-encoded. Check the index (and, for a DataFrame, the columns) dtype as well.
45c681f to
cc092fb
Compare
|
Went ahead and updated the affected tests so CI is green. The blast radius turned out to be smaller than my first estimate — it's exactly six functions, all in I kept these as epoch round-trip tests and wrapped the Happy to flip any of these to |
| ser = ser.astype(object) | ||
| warn = None |
There was a problem hiding this comment.
Can you instead do ser.index = ser.index.astype(object) so we are actually testing with object dtype?
There was a problem hiding this comment.
Can you still make this change?
| - :meth:`HDFStore.put` and :meth:`HDFStore.append` now support storing :class:`Series` and :class:`DataFrame` columns with :class:`PeriodDtype` in both ``"fixed"`` and ``"table"`` formats (:issue:`41978`) | ||
| - Bug in :meth:`DataFrame.__repr__` raising ``TypeError`` for a column with a NumPy structured dtype (e.g. produced by :meth:`DataFrame.from_records` from a structured ``ndarray``) (:issue:`55011`) | ||
| - Bug in :meth:`DataFrame.__repr__` where horizontally truncated output could exceed the terminal width by up to 4 characters (:issue:`32461`) | ||
| - Bug in :meth:`DataFrame.to_json` and :meth:`Series.to_json` not emitting the default ``date_format="epoch"`` deprecation warning when the datetime-like values were in the index or column labels rather than in the column values (:issue:`65868`) |
There was a problem hiding this comment.
Can you move this to the deprecations section?
|
Thanks @jorisvandenbossche! Moved the note to the Deprecations section. On the test suggestion ( ser = Series([Timedelta(milliseconds=42)]).astype(object)
ser.index = ser.index.astype(object)
ser.to_json() # -> {"42":42} (epoch-encoded), but no Pandas4WarningThe reason is that this PR detects datetime-likes via DataFrame({"a": [Timestamp("2020-01-01")]}, dtype=object).to_json()
# -> {"a":{"0":1577836800000}} (epoch), no warningSo warning on object-dtype datetime-like axes is a real, slightly broader gap. I can extend the check to fall back to
Happy to extend it here (I kept the test on the native |
Yes, I had noticed that as well. I was planning to open a separate issue about this to fix. Your suggestion to use But let's make that a focused change in a follow-up PR. |
|
Sounds good, agreed on keeping it focused. This PR stays on the native datetime-like dtypes (the test index remains |
Only convert the index to object dtype, keeping the values as timedelta (m8[ns]). This better isolates the GH#65868 behavior — the epoch deprecation warning should fire when the index contains datetime-like values even with object-dtype index. Addresses review feedback from jorisvandenbossche on pandas-dev#65869. Signed-off-by: Vincent Gao <[email protected]>
binggao1230
left a comment
There was a problem hiding this comment.
Thanks for the review! I've updated the test to use ser.index = ser.index.astype(object) instead of ser = ser.astype(object), so we're testing with object-dtype index specifically. The whatsnew entry was already moved to the Deprecations section in an earlier commit.
…poch-index-warning
|
Thanks @gaoflow |
doc/source/whatsnew/v3.1.0.rstfile if fixing a bugProblem
to_json()warns about the deprecated defaultdate_format="epoch"only when a datetime-like value is in the column values. When the datetime is in the index (or the column labels), the output still contains epoch integers but no warning is emitted:As @jorisvandenbossche notes in the issue, to switch the default to
"iso"in 4.0 we need to warn in both cases now.Cause
In
NDFrame.to_jsonthe warning condition only inspects the column-value dtypes (self.dtypes/self.dtype). Butpandas/io/json/_json.pyepoch-encodes the index and the column labels too, so those axes can produce epoch output without triggering the warning.Fix
Check the index dtype as well (and, for a
DataFrame, the columns dtype) when deciding whether to emit the deprecation warning. Behaviour for the existing column-value case is unchanged, and no double warning is produced.The existing parametrized test
test_default_epoch_date_format_deprecatedis extended with a datetime index, a timedelta index, datetime column labels, and aSerieswith a datetime index.