Skip to content

BUG: json invoke default handler for unsupported numpy dtypes #12878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

Komnomnomnom
Copy link
Contributor

@Komnomnomnom Komnomnomnom commented Apr 12, 2016

Recommend to merge after #12802 has been accepted. (valgrind should be clean then)

@@ -203,6 +203,7 @@ Performance Improvements



- Bug in ``DataFrame.to_json`` with unsupported `dtype` not passed to default handler (:issue:`12554`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should go in bug fix section

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@jreback
Copy link
Contributor

jreback commented Apr 26, 2016

ok so other PR merged, you can rebase

@jreback jreback added this to the 0.18.1 milestone Apr 26, 2016
@Komnomnomnom
Copy link
Contributor Author

FYI windows test https://ci.appveyor.com/project/Komnomnomnom/pandas/build/1.0.4 . Thanks for the tip!

@jreback
Copy link
Contributor

jreback commented Apr 26, 2016

@Komnomnomnom gr8!. note that appveyor is REALLY SLOW!

@Komnomnomnom Komnomnomnom force-pushed the np-default-handler branch 2 times, most recently from 5cf2147 to ebf77e3 Compare April 27, 2016 12:40

from datetime import timedelta
dftd = DataFrame([timedelta(23), timedelta(seconds=5), 42])
* check if the object has defined a ``toDict`` method and call it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did toDict come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ujson. Has been there since the beginning

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, ok, so its 'standard'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am 👍 on deprecating / removing toDict and the object traversal behaviour.

Copy link
Contributor Author

@Komnomnomnom Komnomnomnom Apr 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik ujson is the only library that supports toDict and fallback to object traversal (i.e. serialising obj.__dict__)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, go ahead and deprecate and use obj.__dict__

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear I'm suggesting deprecating toDict and the fallback to obj.__dict__

So default_handler would be the only fallback behaviour (and possibly a future default_enc)

Copy link
Contributor

@jreback jreback Apr 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I c.

for sure deprecate toDict().

is there something problematic with obj.__dict__? e.g. maybe as a fallback to default_handler (which you would use first, or if None then use obj.__dict__).

actually on second though. __dict__ is internal, however, you could do dict(obj) to coerce (and use its output).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation uses dir(obj) and just encodes 'public' attrs. It works ok sometimes.

I don't have strong feelings on removing it but In my experience unless the objects are very simple it's generally not what you want and it's also quite brittle (OverflowErrors are common).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, with deprecating non-supported things and just using default_handler that's what its there for. in theory using __json__ might be ok, but not really sure what is the 'standard'.

@Komnomnomnom
Copy link
Contributor Author

@jreback build clean (apart from msgpack & pickle again)
https://travis-ci.org/Komnomnomnom/pandas/builds/126098431

also windows is ok:
https://ci.appveyor.com/project/Komnomnomnom/pandas/build/1.0.8

@jreback
Copy link
Contributor

jreback commented Apr 27, 2016

just remember how to fix the tag issue.

on your local master
git push yourorigin master --tags

tags come from your fork (and not from the clone, pandas)

next time you branch you should be good

@jreback jreback closed this in 8001e15 Apr 27, 2016
@jreback
Copy link
Contributor

jreback commented Apr 27, 2016

thanks!

make an issue about the deprecations for 0.19.0 pls.

@Komnomnomnom Komnomnomnom deleted the np-default-handler branch April 27, 2016 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Complex Complex Numbers Enhancement IO JSON read_json, to_json, json_normalize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

to_json raises 'Unhandled numpy dtype 15' with complex values.
2 participants