Skip to content

Commit

Permalink
refactor: remove unused flatten function (#20582)
Browse files Browse the repository at this point in the history
  • Loading branch information
zhaoyongjie committed Jul 1, 2022
1 parent 290b89c commit b870a21
Show file tree
Hide file tree
Showing 17 changed files with 28 additions and 192 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/caches.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const assetsConfig = {
path: [`${workspaceDirectory}/superset/static/assets`],
hashFiles: [
`${workspaceDirectory}/superset-frontend/src/**/*`,
`${workspaceDirectory}/superset-frontend/packages/**/*`,
`${workspaceDirectory}/superset-frontend/plugins/**/*`,
`${workspaceDirectory}/superset-frontend/*.js`,
`${workspaceDirectory}/superset-frontend/*.json`,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ export const pivotOperator: PostProcessingFactory<PostProcessingPivot> = (
metricLabels.map(metric => [metric, { operator: 'mean' }]),
),
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ export const timeComparePivotOperator: PostProcessingFactory<PostProcessingPivot
index,
columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
aggregates,
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ test('pivot by __timestamp without groupby', () => {
'sum(val)': { operator: 'mean' },
},
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
},
});
});
Expand All @@ -103,8 +101,6 @@ test('pivot by __timestamp with groupby', () => {
'sum(val)': { operator: 'mean' },
},
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
},
});
});
Expand All @@ -131,8 +127,6 @@ test('pivot by x_axis with groupby', () => {
'sum(val)': { operator: 'mean' },
},
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
},
});
});
Expand Down Expand Up @@ -163,8 +157,6 @@ test('pivot by adhoc x_axis', () => {
'sum(val)': { operator: 'mean' },
},
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
},
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ const queryObject: QueryObject = {
},
},
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ test('should pivot on any type of timeCompare', () => {
},
},
drop_missing_columns: false,
flatten_columns: false,
reset_index: false,
columns: ['foo', 'bar'],
index: ['__timestamp'],
},
Expand Down Expand Up @@ -133,8 +131,6 @@ test('should pivot on x-axis', () => {
drop_missing_columns: false,
columns: ['foo', 'bar'],
index: ['ds'],
flatten_columns: false,
reset_index: false,
},
});
});
Expand Down Expand Up @@ -174,8 +170,6 @@ test('should pivot on adhoc x-axis', () => {
drop_missing_columns: false,
columns: ['foo', 'bar'],
index: ['my_case_expr'],
flatten_columns: false,
reset_index: false,
},
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,10 @@ interface _PostProcessingPivot {
columns: string[];
combine_value_with_metric?: boolean;
drop_missing_columns?: boolean;
flatten_columns?: boolean;
index: string[];
marginal_distribution_name?: string;
marginal_distributions?: boolean;
metric_fill_value?: any;
reset_index?: boolean;
};
}
export type PostProcessingPivot = _PostProcessingPivot | DefaultPostProcessing;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ const PIVOT_RULE: PostProcessingPivot = {
index: ['foo'],
columns: ['bar'],
aggregates: AGGREGATES_OPTION,
flatten_columns: true,
reset_index: true,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@ test('should compile query object A', () => {
},
columns: ['foo'],
drop_missing_columns: false,
flatten_columns: false,
index: ['__timestamp'],
reset_index: false,
},
},
{
Expand Down Expand Up @@ -188,9 +186,7 @@ test('should compile query object B', () => {
},
columns: [],
drop_missing_columns: false,
flatten_columns: false,
index: ['__timestamp'],
reset_index: false,
},
},
{
Expand Down Expand Up @@ -314,9 +310,7 @@ test('should compile query objects with x-axis', () => {
},
columns: ['foo'],
drop_missing_columns: false,
flatten_columns: false,
index: ['my_index'],
reset_index: false,
},
},
{
Expand Down Expand Up @@ -354,9 +348,7 @@ test('should compile query objects with x-axis', () => {
},
columns: [],
drop_missing_columns: false,
flatten_columns: false,
index: ['my_index'],
reset_index: false,
},
},
{
Expand Down
2 changes: 0 additions & 2 deletions superset/utils/pandas_postprocessing/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
from superset.utils.pandas_postprocessing.rolling import rolling
from superset.utils.pandas_postprocessing.select import select
from superset.utils.pandas_postprocessing.sort import sort
from superset.utils.pandas_postprocessing.utils import _flatten_column_after_pivot

__all__ = [
"aggregate",
Expand All @@ -53,5 +52,4 @@
"select",
"sort",
"flatten",
"_flatten_column_after_pivot",
]
13 changes: 0 additions & 13 deletions superset/utils/pandas_postprocessing/pivot.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from superset.constants import NULL_STRING, PandasAxis
from superset.exceptions import InvalidPostProcessingError
from superset.utils.pandas_postprocessing.utils import (
_flatten_column_after_pivot,
_get_aggregate_funcs,
validate_column_args,
)
Expand All @@ -40,8 +39,6 @@ def pivot( # pylint: disable=too-many-arguments,too-many-locals
combine_value_with_metric: bool = False,
marginal_distributions: Optional[bool] = None,
marginal_distribution_name: Optional[str] = None,
flatten_columns: bool = True,
reset_index: bool = True,
) -> DataFrame:
"""
Perform a pivot operation on a DataFrame.
Expand All @@ -61,8 +58,6 @@ def pivot( # pylint: disable=too-many-arguments,too-many-locals
:param marginal_distributions: Add totals for row/column. Default to False
:param marginal_distribution_name: Name of row/column with marginal distribution.
Default to 'All'.
:param flatten_columns: Convert column names to strings
:param reset_index: Convert index to column
:return: A pivot table
:raises InvalidPostProcessingError: If the request in incorrect
"""
Expand Down Expand Up @@ -114,12 +109,4 @@ def pivot( # pylint: disable=too-many-arguments,too-many-locals
if combine_value_with_metric:
df = df.stack(0).unstack()

# Make index regular column
if flatten_columns:
df.columns = [
_flatten_column_after_pivot(col, aggregates) for col in df.columns
]
# return index as regular column
if reset_index:
df.reset_index(level=0, inplace=True)
return df
28 changes: 2 additions & 26 deletions superset/utils/pandas_postprocessing/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
# specific language governing permissions and limitations
# under the License.
from functools import partial
from typing import Any, Callable, Dict, Tuple, Union
from typing import Any, Callable, Dict

import numpy as np
import pandas as pd
from flask_babel import gettext as _
from pandas import DataFrame, NamedAgg, Timestamp
from pandas import DataFrame, NamedAgg

from superset.exceptions import InvalidPostProcessingError

Expand Down Expand Up @@ -97,30 +97,6 @@
FLAT_COLUMN_SEPARATOR = ", "


def _flatten_column_after_pivot(
column: Union[float, Timestamp, str, Tuple[str, ...]],
aggregates: Dict[str, Dict[str, Any]],
) -> str:
"""
Function for flattening column names into a single string. This step is necessary
to be able to properly serialize a DataFrame. If the column is a string, return
element unchanged. For multi-element columns, join column elements with a comma,
with the exception of pivots made with a single aggregate, in which case the
aggregate column name is omitted.
:param column: single element from `DataFrame.columns`
:param aggregates: aggregates
:return:
"""
if not isinstance(column, tuple):
column = (column,)
if len(aggregates) == 1 and len(column) > 1:
# drop aggregate for single aggregate pivots with multiple groupings
# from column name (aggregates always come first in column name)
column = column[1:]
return FLAT_COLUMN_SEPARATOR.join([str(col) for col in column])


def _is_multi_index_on_columns(df: DataFrame) -> bool:
return isinstance(df.columns, pd.MultiIndex)

Expand Down
2 changes: 0 additions & 2 deletions tests/unit_tests/pandas_postprocessing/test_compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ def test_compare_after_pivot():
"sum_metric": {"operator": "sum"},
"count_metric": {"operator": "sum"},
},
flatten_columns=False,
reset_index=False,
)
"""
count_metric sum_metric
Expand Down
4 changes: 0 additions & 4 deletions tests/unit_tests/pandas_postprocessing/test_cum.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ def test_cum_after_pivot_with_single_metric():
index=["dttm"],
columns=["country"],
aggregates={"sum_metric": {"operator": "sum"}},
flatten_columns=False,
reset_index=False,
)
"""
sum_metric
Expand Down Expand Up @@ -127,8 +125,6 @@ def test_cum_after_pivot_with_multiple_metrics():
"sum_metric": {"operator": "sum"},
"count_metric": {"operator": "sum"},
},
flatten_columns=False,
reset_index=False,
)
"""
count_metric sum_metric
Expand Down
Loading

0 comments on commit b870a21

Please sign in to comment.