[ENH] Use default EA repr for IntervalArray#26316
[ENH] Use default EA repr for IntervalArray#26316jschendel merged 23 commits intopandas-dev:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26316 +/- ##
===========================================
- Coverage 92.04% 40.74% -51.31%
===========================================
Files 175 175
Lines 52291 52291
===========================================
- Hits 48132 21306 -26826
- Misses 4159 30985 +26826
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26316 +/- ##
=========================================
Coverage ? 91.97%
=========================================
Files ? 180
Lines ? 50774
Branches ? 0
=========================================
Hits ? 46701
Misses ? 4073
Partials ? 0
Continue to review full report at Codecov.
|
|
I'm not sure we need / want to make this change. |
|
Also not sure if we need/want this change, as it adds an additional layer of complexity to maintaining common docstrings. Certainly something we could work around if we want to change the repr but not ideal. cc @jorisvandenbossche : what are your thoughts here? |
jschendel
left a comment
There was a problem hiding this comment.
There are a some additional IntervalArray docstrings that would need to be updated to match the new repr that aren't currently being checked by CI, e.g. from_arrays, from_breaks, etc.
pandas/pandas/core/arrays/interval.py
Lines 252 to 257 in bb0376c
| self.right.equals(other.right) and | ||
| self.closed == other.closed) | ||
|
|
||
| @Appender(_interval_shared_docs['overlaps'] % _index_doc_kwargs) |
There was a problem hiding this comment.
I think there should be a way of reusing the main portion of the docstring, and appending in custom examples. Something similar to the constructor maybe?
pandas/pandas/core/indexes/interval.py
Lines 100 to 124 in bb0376c
|
Personally I think in principle it would be nice to start using the same repr style for all Array classes. |
|
@TomAugspurger what is your objection here? this de-special cases IntervalArray repr and makes it like all other Arrays, a good thing. |
|
Having to rewrite the docstrings to pass doctests.
… On May 11, 2019, at 14:16, Jeff Reback ***@***.***> wrote:
@TomAugspurger what is your objection here? this de-special cases IntervalArray repr and makes it like all other Arrays, a good thing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@makbigc can you merge master. |
|
working. Give me some more time. |
23322a6 to
c7121a7
Compare
|
@jschendel Amended. Please tell me if anything is wrong. |
|
ok this looks pretty good now; @jschendel |
jschendel
left a comment
There was a problem hiding this comment.
Can you add a whatsnew note with previous/current behavior examples to the 1.0 whatsnew since this is technically a backwards incompatible change?
|
Will review this in more depth later tonight but looks good overall. Would be nice to have an automated way of converting between |
|
|
||
| .. ipython:: python | ||
|
|
||
| In [2]: pd.arrays.IntervalArray.from_tuples([(0, 1), (2, 3)]) |
There was a problem hiding this comment.
Since this is an actual python block I don't think you need the In/Out - that may be causing the CI failure
There was a problem hiding this comment.
@WillAyd
Thanks for your advice. Otherwise I can't pass the CI tests.
|
@WillAyd @jschendel Anything else? |
|
|
||
| *pandas 1.0.0* | ||
|
|
||
| .. code-block:: ipython |
There was a problem hiding this comment.
Sorry could have been clearer but just make this `.. ipython:: python`` for the directive. You shouldn't then need the In / Out below and it will render automatically for you (this is typically what we do with new behavior)
|
|
||
| *pandas 1.0.0* | ||
|
|
||
| .. code-block:: python |
There was a problem hiding this comment.
In case it helps you should use code-block for the old behavior and the ipython directive for newly introduced behavior. You can see an example of this with the Better Repr for MultiIndex section back in the 0.25 whatsnew
|
thanks @makbigc! |
git diff upstream/master -u -- "*.py" | flake8 --diff