Skip to content
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

REF/API: EA formatting methods #55426

Open
jbrockmendel opened this issue Oct 6, 2023 · 5 comments
Open

REF/API: EA formatting methods #55426

jbrockmendel opened this issue Oct 6, 2023 · 5 comments
Assignees
Labels
API - Consistency Internal Consistency of API/Behavior Refactor Internal refactoring of code

Comments

@jbrockmendel
Copy link
Member

jbrockmendel commented Oct 6, 2023

I went down a rabbit hole trying to unravel the many methods we have for rendering arrays. The status quo is not great (but not urgent). The methods I'm looking at are:

  • (EA|Index|Series|Dataframe).__repr__
  • Index.format (xref DEPR: Index.format? #55413)
  • (Series|DataFrame).to_string, to_html, to_latex, (maybe other Styler-like things im not familiar with?)
  • to_csv
  • to_json
  • (havent really looked at to_xml, to_stata, to_hdf, to_excel etc)

The pain points are roughly:

  1. We special-case our internal EAs in ways that complicate the code and make it difficult to reason about. Some of these are just for perf, others actually break tests if we remove the special casing.
  2. Keywords specific to dt64/td64 dtypes are used with our numpy dtypes but not for pyarrow dtypes or 3rd party dtypes. In particular I'm thinking of date_format in to_csv and in DatetimeIndexOpsMixin.format (xref BUG: ExtensionArray formatting of datetime-like forces nanosecond precision #33319)
  3. The boxed keyword in EA._formatter is documented as being True when rendering an EA inside a Index/Series/DataFrame, but the way it is enabled is via fallback_formatter in format_array may use it or not depending on spaghetti logic. Also for dt64/td64/period we dont box the values in Series/DataFrame but do in Index/EA.
    • AFAICT this is largely motivated by the idea that eval(repr(index)) is valid, which i don't particularly care about.
  4. Many of the code paths cast to object in ways that look unnecessary.
  5. _Timedelta64Formatter and _Datetime64Formatter have a nat_rep keyword in __init__ that is never passed. The caller format_array does pass na_rep (which defaults to "NaN" and using it expect would break a zillion tests)
  6. to_json just doesn't work with general EA dtypes (xref to_json/read_json can't handle interval index #35420, to_json of Series with period dtype results in AttributeError #31917, JSON table orient not roundtripping extension types #32037)
  7. date_format in to_json behaves differently from everywhere else (xref No way with to_json to write only date out of datetime #16492, Request to add more date formats in to_json method #22317, ENH: Add support for date_unit to be specified per column in to_json #39135, ENH: Add new date_format option to_json matching datetime.isoformat exactly #47930)
  8. General hodgepodge of mismatched keywords in different to_foo methods (most of which is unavoidable)

Some ideas on improving the situation:

A) Deprecate Index.format (xref #55413) and implement Index._format_flat and Index._format_multi for internal use. We don't use most of the existing keywords internally, so the new methods would be appreciably simpler than the current ones.
B) add relevant keywords (float_format, decimal, date_format) to EA._formatter to make the fallback_formatter spaghetti in 3) unnecessary
C) add a vectorized formatting method to EA with the same behavior as our internal EAs' _format_native_values. Then avoid special-casing our internal EAs in the relevant places.
      i) Note we used to have EA._formatting_values and i guess that was removed in favor of just _formatter. I think we could implement the default _vectorized_whatever in terms of _formatter so the new method would be optional for 3rd party authors.
      ii) The "relevant places" would include blocks.to_native_types (used in to_csv), to_json, Index._format_native_values, and format_array
D) Change the boxed behavior to box in when inside and Index but not in Series/DataFrame.
E) for 5, just accept/document that na_rep is ignored for these dtypes and you'll always get "NaT"
F) standardize the "date_format" behavior across to_json and other places where it shows up

@jbrockmendel jbrockmendel added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Oct 6, 2023
@lithomas1 lithomas1 added Refactor Internal refactoring of code API - Consistency Internal Consistency of API/Behavior and removed Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Oct 6, 2023
@Deadlica
Copy link

take

@Gustaf-Larsson
Copy link

take

@andrefred
Copy link

take

@raeef96
Copy link

raeef96 commented Feb 28, 2024

take

@NoelMT NoelMT removed their assignment Mar 4, 2024
@tomytp
Copy link
Contributor

tomytp commented May 12, 2024

take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API - Consistency Internal Consistency of API/Behavior Refactor Internal refactoring of code
Projects
None yet
Development

No branches or pull requests

8 participants