There's an old TODO note about untested code in the extension module hanging around in objToJSON.c:
https://github.com/pandas-dev/pandas/blob/bfff080275b4456b28d71f0c7b4ec9e678d4270c/pandas/_libs/src/ujson/python/objToJSON.c#L504
Interestingly enough, this is actually tested by the np.datetime64
parameter in test_encode_as_null
as part of the test_ujson.py
module:
https://github.com/pandas-dev/pandas/blob/bfff080275b4456b28d71f0c7b4ec9e678d4270c/pandas/tests/io/json/test_ujson.py#L388
The intent here is somewhat blurry. I think the test_ujson.py
module was created when it was up for discussion if we wanted the JSON serializers to be publicly exposed at the top level (see #9147) which I don't believe is still something we want to do. At that time it would make sense to explicitly support the np.datetime64
type.
The remaining question then is whether np.datetime64
should be represented within a pandas container. It seems that this can be held in a Series with an object dtype but not a DataFrame, as shown below:
>>> ser = pd.Series([np.datetime64("2000-01-01")], dtype=object)
>>> type(ser.iloc[0])
numpy.datetime64
>>> type(ser.to_frame().iloc[0, 0])
pandas._libs.tslibs.timestamps.Timestamp
If we don't want np.datetime64
objects to be contained then we can delete this code altogether. If we do we should add test coverage for it, but then also address the inconsistency across container types above
Comment From: jbrockmendel
Interestingly enough, this is actually tested by the np.datetime64 parameter in test_encode_as_null as part of the test_ujson.py module:
I'm pretty sure I added that TODO note. IIRC I commented out the relevant code and found that it didn't break any tests. So its conceivable that it is reached in a test, but it is a smoke-test?
if the note is just inaccurate, feel free to remote it.
Comment From: WillAyd
Maybe commenting has a different effect but it definitely causes a failure if outright removed in test_encode_as_null
from test_ujson.py
when dealing with an np.datetime64 object
The only way to really add a test for this would be to create a Series with a np.datetime64 item and object
dtype; any other path I think implicitly converts the np.datetime64
to a pandas object.
So could add that Series test though not sure what the overall intent of date time handling is
Comment From: Karthik-PM
can i work on this issue
Comment From: mroeschke
Looks like this code doesn't exist anymore in objToJSON, so guess we can close this one out