COMPAT: restore shape for 'invalid' Index with nd array by jorisvandenbossche · Pull Request #27818 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation12 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tacaswell will matplotlib be able to track down the places that need updating, to cast Index objects to an ndarray earlier? Would it help if we put an (invisible by default) DeprecationWarning in here?
| @@ -111,7 +111,7 @@ Plotting |
|---|
| ^^^^^^^^ |
| - Added a pandas_plotting_backends entrypoint group for registering plot backends. See :ref:`extending.plotting-backends` for more (:issue:`26747`). |
| - |
| - Fix compatibility issue with matplotlib when passing a pandas ``Index`` to a plot call (:issue:`27775`). |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that this is vague about the underlying issue. I don't want people relying on Index.shape being n-d.
Would it help if we put an (invisible by default) DeprecationWarning in here?
What would you deprecate? It's creating the 2D Index that we should deprecate, not necessarily accessing its shape.
But deprecating that should go on master, not for 0.25.1 I think.
To be clear, I only see this as a fix for 0.25.1, and we should further discuss how we want to fix this long term on master (convert to ndarray, or deprecate/raise error)
What would you deprecate?
Not really a deprecation. Just a "hey this is wrong" warning that's invisible by default in Index.shape. I just don't know how matplotlib is going to catch this otherwise (though maybe they have a central place where they do their conversion to NumPy).
To be clear, do you propose that instead of this PR? (or as something that should be added to this PR?)
(for me that is again part of a separate longer term solution discussion)
seems ok to me for 0.25.1, I think we should raise starting in 1.0.0 for this on consruction; its invalid.
To be clear, do you propose that instead of this PR? (or as something that should be added to this PR?)
Not instead, in addition to. Let's see how the index construction goes.
@jreback What type would you raise? If it is TypeError or IndexError it will "just work" with Matplotilb.
Turns out this used to be extra goofy:
In [13]: import pandas
In [14]: pandas.version Out[14]: '0.24.2'
In [15]: import pandas as pd
In [16]: df = pd.DataFrame([1, 2])
In [17]: df.index[:, None].shape Out[17]: (2, 1)
In [18]: df.index[:, None].ndim Out[18]: 1
@tacaswell yes, it was buggy, it is still buggy, we just changed the way it was buggy (and this PR restores the previous way of buggy)
But let's keep all that discussion about additional fixes, returning array or erroring, .. for a proper issue about it (I will open one).
meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request
quintusdias pushed a commit to quintusdias/pandas_dev that referenced this pull request
Labels
pandas objects compatability with Numpy or Python functions