REF: move values_for_json to EA #53680
Conversation
pandas/core/arrays/base.py
Outdated
| @@ -1443,6 +1443,10 @@ def _reduce(self, name: str, *, skipna: bool = True, **kwargs): | |||
| # Non-Optimized Default Methods; in the case of the private methods here, | |||
| # these are not guaranteed to be stable across pandas versions. | |||
|
|
|||
| def _values_for_json(self) -> np.ndarray: | |||
| # TODO: document! | |||
There was a problem hiding this comment.
Yeah would be good to have this before merging
There was a problem hiding this comment.
@WillAyd thoughts on what would be useful here?
There was a problem hiding this comment.
Guessing this should just be an object dtype array where any value is JSON serializable (ex: string, number, list, dict)
There was a problem hiding this comment.
the status quo returns non-object ndarrays in many cases
There was a problem hiding this comment.
Hmm yea but that is also because the JSON serializer has extra logic to handle primitives for those types (eg: int64 backing for datetime). Is the idea here that general EAs need to define this or is this scoped to the internally managed EAs?
There was a problem hiding this comment.
The real motivation is to try to move EA-specific logic from Blocks to EAs. I'm kind of hoping that we can stubmle onto a solution to #31917 along the way
There was a problem hiding this comment.
Gotcha. For that issue @lithomas1 might be interested in implementing the serializer.
As far as this PR is concerned I am ok with it either way. Generally not sure it should end up in the EA in the long run, but doesn't hurt to move around today either
mroeschke
added
IO JSON
|
Added docstring. Not confident enough in how this works to add tests yet |
|
@WillAyd OK here? |
|
I don't know that I see the vision for how this will actually work with JSON arguments (i.e. date formatting) but no objection to it either |
|
Unless there are more comments, I'm planning to merge later this week. |
|
Thanks @jbrockmendel |
* REF: move values_for_json to EA * docstring
* REF: move values_for_json to EA * docstring Co-authored-by: jbrockmendel <[email protected]>
* REF: move values_for_json to EA * docstring
* REF: move values_for_json to EA * docstring Co-authored-by: jbrockmendel <[email protected]>
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.