REF: remove ExtensionArrayFormatter
State: closed

- [ ] precursor #26000
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Refactor
description: Internal refactoring of code
name: Output-Formatting
description: __repr__ of pandas objects, to_string

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=h1) Report
> Merging [#26833](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/e8132a2901075b96b0d6e74639d5c341bec115fe?src=pr&el=desc) will **increase** coverage by `1.39%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/26833/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26833      +/-   ##
==========================================
+ Coverage   90.45%   91.85%   +1.39%     
==========================================
  Files         179      179              
  Lines       50706    50700       -6     
==========================================
+ Hits        45866    46570     +704     
+ Misses       4840     4130     -710
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.44% <100%> (-0.01%)` | :arrow_down: |
| #single | `41.12% <20%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/formats/format.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvZm9ybWF0LnB5) | `97.89% <100%> (-0.02%)` | :arrow_down: |
| [pandas/core/indexes/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9pbnRlcnZhbC5weQ==) | `96.11% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2Jhc2UucHk=) | `98.87% <0%> (-0.57%)` | :arrow_down: |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `98.3% <0%> (-0.25%)` | :arrow_down: |
| [pandas/core/arrays/sparse.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3NwYXJzZS5weQ==) | `93.59% <0%> (-0.12%)` | :arrow_down: |
| [pandas/core/arrays/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2NhdGVnb3JpY2FsLnB5) | `95.8% <0%> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.94% <0%> (+0.1%)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `96.37% <0%> (+0.16%)` | :arrow_up: |
| [pandas/io/formats/printing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvcHJpbnRpbmcucHk=) | `86.09% <0%> (+1.6%)` | :arrow_up: |
| [pandas/io/clipboard/clipboards.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2NsaXBib2FyZC9jbGlwYm9hcmRzLnB5) | `34.78% <0%> (+2.89%)` | :arrow_up: |
| ... and [4 more](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=footer). Last update [e8132a2...a777fa4](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=h1) Report
> Merging [#26833](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/e8132a2901075b96b0d6e74639d5c341bec115fe?src=pr&el=desc) will **increase** coverage by `1.39%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/26833/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26833      +/-   ##
==========================================
+ Coverage   90.45%   91.85%   +1.39%     
==========================================
  Files         179      179              
  Lines       50706    50700       -6     
==========================================
+ Hits        45866    46570     +704     
+ Misses       4840     4130     -710
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.44% <100%> (-0.01%)` | :arrow_down: |
| #single | `41.12% <20%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/formats/format.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvZm9ybWF0LnB5) | `97.89% <100%> (-0.02%)` | :arrow_down: |
| [pandas/core/indexes/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9pbnRlcnZhbC5weQ==) | `96.11% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2Jhc2UucHk=) | `98.87% <0%> (-0.57%)` | :arrow_down: |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `98.3% <0%> (-0.25%)` | :arrow_down: |
| [pandas/core/arrays/sparse.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3NwYXJzZS5weQ==) | `93.59% <0%> (-0.12%)` | :arrow_down: |
| [pandas/core/arrays/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2NhdGVnb3JpY2FsLnB5) | `95.8% <0%> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.94% <0%> (+0.1%)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `96.37% <0%> (+0.16%)` | :arrow_up: |
| [pandas/io/formats/printing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvcHJpbnRpbmcucHk=) | `86.09% <0%> (+1.6%)` | :arrow_up: |
| [pandas/io/clipboard/clipboards.py](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2NsaXBib2FyZC9jbGlwYm9hcmRzLnB5) | `34.78% <0%> (+2.89%)` | :arrow_up: |
| ... and [4 more](https://codecov.io/gh/pandas-dev/pandas/pull/26833/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=footer). Last update [e8132a2...a777fa4](https://codecov.io/gh/pandas-dev/pandas/pull/26833?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ok this is fine then. I agree, think need to refactor how ``format_array`` works, @simonjayhawkins maybe make an issue for it? to push more functionaily to the objects (and PandasArray) rather than encoding the conversions where they are.
and we need to get rid of that leading_space business too?
> and we need to get rid of that leading_space business too?

yes I think that's right
> i should have a follow-on ready shortly continuing the format_array cleanup. (maybe not till next week due to PyLondinium)

I would then propose to revert this PR, and do a proper refactor all at once using this `_formatter` method in the Formatter class as you mention. 
We might also need to think about if we need to add additonal arguments to `_formatter` to enable some of the options (but didn't look enough in detail to the formatting code to assess this)

The `_formatter` is essential to let EA authors override the formatting (and eg now master will break what I am working on in GeoPandas). It's a bit unfortunate we didn't have proper tests for this.
> The `_formatter` is essential to let EA authors override the formatting (and eg now master will break what I am working on in GeoPandas). It's a bit unfortunate we didn't have proper tests for this.

EA authors would therefore also need to ensure that additional formatting options can be honoured, eg max_colwidth, float_format, na_rep etc. Should this be their responsibility?
Well, that's indeed something to be discussed how to do that (that's what I meant above with looking into adding additional arguments to `_formatter`). 

We probably should have done that thought exercise when introducing the `_formatter` method, but that's too late now and the `_formatter` is in the EA interface, so we should honor it (at least for external EAs).
a `formatters` argument is in some of the to_* methods api, and we need to honour that too.

i'm OK if you want to revert for now though.
I added a revert in the PR where I am adding a test: https://github.com/pandas-dev/pandas/pull/26845
Is there already an issue to have the general discussion of how to deal with formatting options and ExtensionArrays ?
> I added a revert in the PR where I am adding a test: #26845

no problem. i'll approve on green.

> Is there already an issue to have the general discussion of how to deal with formatting options and ExtensionArrays ?

i'll open a master tracker where we can add things like the Int64 na_rep issue.
> i'll open a master tracker where we can add things like the Int64 na_rep issue.

@simonjayhawkins  where is the discussion issue? I ran into another issues with formatters I'd like to discuss before moving forward with this there's a suggestion/feature request I'd like to put up.
> > i'll open a master tracker where we can add things like the Int64 na_rep issue.
> 
> @simonjayhawkins where is the discussion issue? I ran into another issues with formatters I'd like to discuss before moving forward with this there's a suggestion/feature request I'd like to put up.

I've not opened a separate issue yet for three reasons

- there may be overlap with #26837
- awaiting the outcome of #26817
- thinking about whether _formatting_values should have been deprecated.

feel free to open a new issue.


> awaiting the outcome of #26817

How is the formatting discussion related to that PR?
We can maybe use https://github.com/pandas-dev/pandas/issues/26837 as the general issue to discuss this?
> > awaiting the outcome of #26817
> 
> How is the formatting discussion related to that PR?

IIUC correctly, before EAs, `format_array` formatted an numpy array. and GenericArrayFormatter expects an numpy array, used mainly for numpy arrays with dtype=object

but we now have a ExtensionArrayFormatter that includes logic to convert to a numpy array

```
        values = self.values
        if isinstance(values, (ABCIndexClass, ABCSeries)):
            values = values._values

        formatter = values._formatter(boxed=True)

        if is_categorical_dtype(values.dtype):
            # Categorical is special for now, so that we can preserve tzinfo
            array = values.get_values()
        else:
            array = np.asarray(values)

        fmt_values = format_array(array,
```

whereas before, _formatting_values was used and conversion to a numpy array looked like

```
    def _format_col(self, i):
        frame = self.tr_frame
        formatter = self._get_formatter(i)
        values_to_format = frame.iloc[:, i]._formatting_values()
        return format_array(values_to_format, formatter,
                            float_format=self.float_format, na_rep=self.na_rep,
                            space=self.col_space, decimal=self.decimal)
```

even though _formatting_values is depecated:

```
    def _formatting_values(self) -> np.ndarray:
        # At the moment, this has to be an array since we use result.dtype
        """
        An array of values to be printed in, e.g. the Series repr

        .. deprecated:: 0.24.0

           Use :meth:`ExtensionArray._formatter` instead.
        """
```

it is still used. but does not necessarily return an numpy array

```
>>> import pandas as pd
>>> df = pd.DataFrame({"c": [2,3,float('nan')]})
>>> df = df.astype("Int64")
>>> df.c._formatting_values()
<IntegerArray>
[2, 3, NaN]
Length: 3, dtype: Int64
```

so when creating a _repr_html_ of a DataFrame which uses _format_col, we pass a `<IntegerArray>` to format_array, which then uses ExtensionArrayFormatter to get the values as an numpy array and then call 
format_array again which then formats the numpy array using GenericArrayFormatter.

It seems a bit convoluted.

i'm not sure why a numpy array is not returned from _formatting_values.

I think the issues are related since the discussions revolve around numpy arrays from extension arrays and formatting probably works best if the formatting is done on a numpy array with am object dtype. performance is probably not an issue for formatting.

@simonjayhawkins That's exactly the discussion we should have. But not in a merged PR :-) 
Can you just put that comment in a new issue? (or in #26837, same for me)
@pilkibun can you move your comment to #26837 as well?
> @pilkibun can you move your comment to #26837 as well?

https://github.com/pandas-dev/pandas/issues/26837#issuecomment-503360850