Added extra info section to EA repr

[TomAugspurger]

Closes #24278

[pep8speaks]

Hello @TomAugspurger! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 07, 2019 at 15:39 Hours UTC

[codecov]

Codecov Report

Merging #24279 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #24279      +/-   ##
==========================================
+ Coverage   92.22%   92.22%   +<.01%     
==========================================
  Files         162      162              
  Lines       51828    51830       +2     
==========================================
+ Hits        47798    47801       +3     
+ Misses       4030     4029       -1

Flag	Coverage Δ
#multiple	90.62% <100%> (ø)	⬆️
#single	43% <50%> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/core/arrays/base.py	97.45% <100%> (+0.03%)	⬆️
pandas/util/testing.py	87.41% <0%> (-0.1%)	⬇️
pandas/io/json/json.py	93.09% <0%> (+0.47%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 040f06f...d96942b. Read the comment docs.

[codecov]

Codecov Report

Merging #24279 into master will increase coverage by 1.62%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #24279      +/-   ##
==========================================
+ Coverage   92.22%   93.84%   +1.62%     
==========================================
  Files         162      166       +4     
  Lines       51828    70087   +18259     
==========================================
+ Hits        47798    65774   +17976     
- Misses       4030     4313     +283

Flag	Coverage Δ
#multiple	92.5% <100%> (+1.87%)	⬆️
#single	46.02% <66.66%> (+3.01%)	⬆️

Impacted Files	Coverage Δ
pandas/core/arrays/base.py	98% <100%> (+0.58%)	⬆️
pandas/core/computation/check.py	87.5% <0%> (-3.41%)	⬇️
pandas/core/groupby/generic.py	86.16% <0%> (-0.96%)	⬇️
pandas/core/strings.py	98.58% <0%> (-0.01%)	⬇️
pandas/core/api.py	100% <0%> (ø)	⬆️
pandas/core/dtypes/generic.py	100% <0%> (ø)	⬆️
pandas/core/internals/__init__.py	100% <0%> (ø)	⬆️
pandas/core/arrays/__init__.py	100% <0%> (ø)	⬆️
pandas/core/arrays/numpy_.py	93.51% <0%> (ø)
pandas/core/arrays/array_.py	100% <0%> (ø)
... and 69 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 040f06f...d96942b. Read the comment docs.

[jreback]

can you do the same for Index

[TomAugspurger]

The same what? This is just changing the base ExtensionArray repr.

[jreback]

The same what? This is just changing the base ExtensionArray repr.

yes, the same, to avoid departing from what we do in Index, which is pretty similar.

[TomAugspurger]

Sorry, I'm still not following what you're suggesting.

[jreback]

https://github.com/pandas-dev/pandas/blob/master/pandas/core/indexes/base.py#L929

this is called _format_attrs in Index, so change one or the other

[TomAugspurger]

IIUC, Index._format_attrs returns a List[Tuple[str, str]]. So things like [('name', self.name), ('dtype', self.dtype)]

ExtensionArray._repr_extra doesn't require key-value pairs. Rather, we just expect a bit of text to include. I don't think it would be appropriate to require key-value pairs here.

[jreback]

IIUC, Index._format_attrs returns a List[Tuple[str, str]]. So things like [('name', self.name), ('dtype', self.dtype)]

ExtensionArray._repr_extra doesn't require key-value pairs. Rather, we just expect a bit of text to include. I don't think it would be appropriate to require key-value pairs here

This is just yet another difference with index. I would rather not have these differences.

[TomAugspurger]

I could imagine a subclass wanting to include something that's not a key-value pair.

[jreback]

@TomAugspurger and that's fine, so change Index. this duplicated code between EA and Index is driving me bananas. The entire point is to share code. So it needs to be changed in one or the other.

[TomAugspurger]

This isn't a priority for me right now. I'd say merge if we think it's an improvement, or close and do it later. Either works for me.

[jreback]

I guess this is fine. ideally just have some integration with Index here (basically use the _repr_extra_ there as well for consistency.

[jorisvandenbossche]

Personally, I would prefer adding the extra section between length and dtype (so dtype is always the last element), but that's minor.

Is it needed to add an extra method for it? Won't we be able to reuse some existing thing like _attributes, and then automatically fill in the value for that?

Should we use this new ability to add freq for DatetimeArray?

[TomAugspurger]

Yes, DatetimeArray should use freq for it.

I don't know which _attributes attribute you're referring to. I don't think there's anything like that on ExtensionArray. DatetimeArray defines _attributes = ['freq', 'tz'], but the tz is already encoded in the dtype.

[jorisvandenbossche]

I don't know which _attributes attribute you're referring to. I don't think there's anything like that on ExtensionArray. DatetimeArray defines _attributes = ['freq', 'tz'], but the tz is already encoded in the dtype.

Ah, yes, I meant that _attributes, but assumed it only had freq in it (exactly because tz is already in the dtype), but didn't actually check ..

[jreback]

looking again. this is reasonable. can you merge master. are there other places that you see this being used in the current EA's? (other than Decimal).

[jreback]

closing this; I think we need more consolidation on repr to avoid code duplication.