-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
TYP: DataFrame.(index|columns) and Series.index #31126
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -134,13 +134,13 @@ def pivot_table( | |
table = agged.unstack(to_unstack) | ||
|
||
if not dropna: | ||
if table.index.nlevels > 1: | ||
if isinstance(table.index, MultiIndex): | ||
m = MultiIndex.from_arrays( | ||
cartesian_product(table.index.levels), names=table.index.names | ||
) | ||
table = table.reindex(m, axis=0) | ||
|
||
if table.columns.nlevels > 1: | ||
if isinstance(table.columns, MultiIndex): | ||
m = MultiIndex.from_arrays( | ||
cartesian_product(table.columns.levels), names=table.columns.names | ||
) | ||
|
@@ -373,7 +373,7 @@ def _generate_marginal_results_without_values( | |
): | ||
if len(cols) > 0: | ||
# need to "interleave" the margins | ||
margin_keys = [] | ||
margin_keys: Union[List, Index] = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could be ArrayLike? (not sure it matters, but can update in followon if this works) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would not work, because |
||
|
||
def _all_key(): | ||
if len(cols) == 1: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,10 +57,13 @@ | |
is_timedelta64_dtype, | ||
) | ||
from pandas.core.dtypes.generic import ( | ||
ABCDatetimeIndex, | ||
ABCIndexClass, | ||
ABCMultiIndex, | ||
ABCPeriodIndex, | ||
ABCSeries, | ||
ABCSparseArray, | ||
ABCTimedeltaIndex, | ||
) | ||
from pandas.core.dtypes.missing import isna, notna | ||
|
||
|
@@ -295,6 +298,9 @@ def _get_footer(self) -> str: | |
footer = "" | ||
|
||
if getattr(self.series.index, "freq", None) is not None: | ||
assert isinstance( | ||
self.series.index, (ABCDatetimeIndex, ABCPeriodIndex, ABCTimedeltaIndex) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't we have a ABCDatetimelike ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, and I only found two instances of this pattern. |
||
) | ||
footer += "Freq: {freq}".format(freq=self.series.index.freqstr) | ||
|
||
if self.name is not False and name is not None: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this is a breaking change?
this PR
on master
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've made it a TypeError, to mirror reorder_levels. I also added a Whatsnew note.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, is this an API change? We're reaching 1.0, so in principle we should be wary on changing the API. Should I raise an AttributeError instead of TypeError to maintain compatibility (though that is strictly a wrong error to raise)?