From f8f23eff9bea81043848abe02f76b9ba1362699d Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Fri, 18 Sep 2026 12:14:03 -0400 Subject: [PATCH 1/4] test(styling): failing tests for header loss on style_column exception (#966) Co-Authored-By: Claude Opus 5 --- tests/unit/dataflow/styling_core_test.py | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/unit/dataflow/styling_core_test.py b/tests/unit/dataflow/styling_core_test.py index cde41ad96..1f455c9db 100644 --- a/tests/unit/dataflow/styling_core_test.py +++ b/tests/unit/dataflow/styling_core_test.py @@ -405,3 +405,30 @@ def test_float_vs_compact_savings(self): compact_w = estimate_min_width_px(compact_disp, 'a', meta) assert compact_w < float_w + + +class RaisingStyling(StylingAnalysis): + requires_summary = [] + + @classmethod + def style_column(cls, col, column_metadata): + raise NameError("boom") + + +def test_failed_style_column_keeps_header_name() -> None: + """A style_column exception falls back to obj styling but keeps the real header (#966).""" + simple_df = pd.DataFrame({'foo': [10, 20, 30], 'bar': ['foo', 'bar', 'baz']}) + dfvc = RaisingStyling.get_dfviewer_config( + {'a': {'orig_col_name': 'foo'}, 'b': {'orig_col_name': 'bar'}}, simple_df) + assert dfvc['column_config'] == [ + {'col_name': 'a', 'header_name': 'foo', 'displayer_args': {'displayer': 'obj'}}, + {'col_name': 'b', 'header_name': 'bar', 'displayer_args': {'displayer': 'obj'}}] + + +def test_failed_style_column_keeps_col_path() -> None: + """Same as above for multi-index columns: the fallback keeps col_path.""" + mic_df = get_multiindex_cols_df() + fake_sd: SDType = {'a': {'orig_col_name': ('foo', 'a')}, 'b': {'orig_col_name': ('foo', 'b')}} + col_config = RaisingStyling.get_dfviewer_config(fake_sd, mic_df)['column_config'] + assert [cc['col_path'] for cc in col_config] == [('foo', 'a'), ('foo', 'b')] + assert [cc['field'] for cc in col_config] == ['a', 'b'] From d4bd8c00b096be86e3a57944b8c752fc4a10d266 Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Fri, 18 Sep 2026 12:20:27 -0400 Subject: [PATCH 2/4] fix(styling): keep real header name when style_column raises (#966) default_styling now takes orig_col_name; style_columns passes it from col_meta so a failing column keeps its header / col_path and only loses its styling. Co-Authored-By: Claude Opus 5 --- buckaroo/dataflow/styling_core.py | 11 +++++++---- tests/unit/dataflow/styling_core_test.py | 6 ++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/buckaroo/dataflow/styling_core.py b/buckaroo/dataflow/styling_core.py index 13b688a8b..e18f03f87 100644 --- a/buckaroo/dataflow/styling_core.py +++ b/buckaroo/dataflow/styling_core.py @@ -1,6 +1,6 @@ import copy import logging -from typing import Iterable, TypedDict, Union, List, Dict, Any, Literal +from typing import Iterable, TypedDict, Union, List, Dict, Any, Literal, Optional from typing_extensions import NotRequired, TypeAlias import pandas as pd @@ -434,8 +434,10 @@ def fix_column_config(cls, col: ColIdentifier, orig_col_name: ColIdentifier, bas summary_stats_key: str = 'all_stats' @classmethod - def default_styling(cls, col_name:Union[Iterable[str], str], /) -> ColumnConfig: - return cls.fix_column_config(col_name, col_name, {'displayer_args': {'displayer': 'obj'}}) + def default_styling(cls, col_name:Union[Iterable[str], str], orig_col_name:Optional[ColIdentifier]=None, /) -> ColumnConfig: + if orig_col_name is None: + orig_col_name = col_name + return cls.fix_column_config(col_name, orig_col_name, {'displayer_args': {'displayer': 'obj'}}) @classmethod def get_dfviewer_config(cls, sd:SDType, df:pd.DataFrame) -> DFViewerConfig: @@ -477,7 +479,8 @@ def style_columns(cls, sd:SDType, df:pd.DataFrame) -> List[ColumnConfig]: # Always provide a style, not providing a style # results in no display which is a very bad user # experience - base_style = cls.default_styling(col) + # keep the real header so a failure only costs this column its styling + base_style = cls.default_styling(col, col_meta.get('orig_col_name')) diff --git a/tests/unit/dataflow/styling_core_test.py b/tests/unit/dataflow/styling_core_test.py index 1f455c9db..5c5569125 100644 --- a/tests/unit/dataflow/styling_core_test.py +++ b/tests/unit/dataflow/styling_core_test.py @@ -432,3 +432,9 @@ def test_failed_style_column_keeps_col_path() -> None: col_config = RaisingStyling.get_dfviewer_config(fake_sd, mic_df)['column_config'] assert [cc['col_path'] for cc in col_config] == [('foo', 'a'), ('foo', 'b')] assert [cc['field'] for cc in col_config] == ['a', 'b'] + + +def test_failed_style_column_without_orig_col_name() -> None: + """Instantiation passes an empty col_meta; the fallback still uses the rewritten id.""" + col_config = RaisingStyling.style_columns({'a': {}}, pd.DataFrame({'foo': [1]})) + assert col_config == [{'col_name': 'a', 'header_name': 'a', 'displayer_args': {'displayer': 'obj'}}] From cf519e13b678e1cd3e32870f176ca176b6d595c1 Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Fri, 18 Sep 2026 15:28:51 -0400 Subject: [PATCH 3/4] test(styling): failing tests for MRO fallback and header identity without orig_col_name (#966) Co-Authored-By: Claude Opus 5 --- tests/unit/dataflow/styling_core_test.py | 78 +++++++++++++++++++++++- 1 file changed, 75 insertions(+), 3 deletions(-) diff --git a/tests/unit/dataflow/styling_core_test.py b/tests/unit/dataflow/styling_core_test.py index 5c5569125..4c1d1ef31 100644 --- a/tests/unit/dataflow/styling_core_test.py +++ b/tests/unit/dataflow/styling_core_test.py @@ -1,7 +1,8 @@ +import copy from typing import Dict, List import pandas as pd from buckaroo.dataflow.styling_core import ColumnConfig, DFViewerConfig, NormalColumnConfig, PartialColConfig, StylingAnalysis, merge_sd_overrides, rewrite_override_col_references -from buckaroo.customizations.styling import (_formatted_char_count, estimate_min_width_px, _HISTOGRAM_MIN_PX, _MIN_COL_PX) +from buckaroo.customizations.styling import (DefaultMainStyling, _formatted_char_count, estimate_min_width_px, _HISTOGRAM_MIN_PX, _MIN_COL_PX) from buckaroo.ddd_library import get_basic_df2, get_multiindex_index_df, get_multiindex_index_multiindex_with_names_cols_df, get_multiindex_index_with_names_multiindex_cols_df, get_multiindex_with_names_both, get_multiindex_with_names_index_df, get_multiindex_cols_df, get_multiindex_with_names_cols_df, get_tuple_cols_df from buckaroo.df_util import ColIdentifier from buckaroo.pluggable_analysis_framework.col_analysis import SDType @@ -435,6 +436,77 @@ def test_failed_style_column_keeps_col_path() -> None: def test_failed_style_column_without_orig_col_name() -> None: - """Instantiation passes an empty col_meta; the fallback still uses the rewritten id.""" + """col_meta without orig_col_name (the instantiation pass) still gets the real header from df.""" col_config = RaisingStyling.style_columns({'a': {}}, pd.DataFrame({'foo': [1]})) - assert col_config == [{'col_name': 'a', 'header_name': 'a', 'displayer_args': {'displayer': 'obj'}}] + assert col_config == [{'col_name': 'a', 'header_name': 'foo', 'displayer_args': {'displayer': 'obj'}}] + + +def test_style_column_without_orig_col_name() -> None: + """Successful styling without orig_col_name must not produce header_name 'None'.""" + col_config = StylingAnalysis.style_columns({'a': {}}, pd.DataFrame({'foo': [1]})) + assert col_config == [{'col_name': 'a', 'header_name': 'foo', 'displayer_args': {'displayer': 'obj'}}] + + +FALLBACK_DF = pd.DataFrame({'foo': [10, 20, 30], 'bar': ['x', 'y', 'z']}) +FALLBACK_SD: SDType = { + 'a': {'orig_col_name': 'foo', '_type': 'integer'}, + 'b': {'orig_col_name': 'bar', '_type': 'string'}} + + +class RaisingMainStyling(DefaultMainStyling): + @classmethod + def style_column(cls, col, column_metadata): + raise NameError("boom") + + +class ColoredMainStyling(DefaultMainStyling): + @classmethod + def style_column(cls, col, column_metadata): + base = super().style_column(col, column_metadata) + base['color_map_config'] = {'color_rule': 'color_static', 'color': 'red'} + return base + + +class RaisingColoredStyling(ColoredMainStyling): + @classmethod + def style_column(cls, col, column_metadata): + raise NameError("boom") + + +class NoneReturningMainStyling(DefaultMainStyling): + @classmethod + def style_column(cls, col, column_metadata): + return None + + +class MutateThenRaiseStyling(DefaultMainStyling): + @classmethod + def style_column(cls, col, column_metadata): + column_metadata['_type'] = 'string' + raise NameError("boom") + + +def test_failed_style_column_falls_back_to_parent() -> None: + """A failing subclass falls back to its parent's style_column, not bare obj.""" + assert RaisingMainStyling.style_columns(FALLBACK_SD, FALLBACK_DF) == \ + DefaultMainStyling.style_columns(FALLBACK_SD, FALLBACK_DF) + + +def test_failed_style_column_falls_back_to_nearest_parent() -> None: + """The fallback walks the MRO, so an intermediate class's styling is kept.""" + assert RaisingColoredStyling.style_columns(FALLBACK_SD, FALLBACK_DF) == \ + ColoredMainStyling.style_columns(FALLBACK_SD, FALLBACK_DF) + + +def test_non_dict_style_column_falls_back_to_parent() -> None: + """Returning something that isn't a column config counts as a failure.""" + assert NoneReturningMainStyling.style_columns(FALLBACK_SD, FALLBACK_DF) == \ + DefaultMainStyling.style_columns(FALLBACK_SD, FALLBACK_DF) + + +def test_fallback_sees_unmutated_col_meta() -> None: + """Edits a failing style_column made to column_metadata don't leak into the fallback or the sd.""" + sd = copy.deepcopy(FALLBACK_SD) + assert MutateThenRaiseStyling.style_columns(sd, FALLBACK_DF) == \ + DefaultMainStyling.style_columns(FALLBACK_SD, FALLBACK_DF) + assert sd == FALLBACK_SD From e3af3083234baf517beb80a4e11157600b30e0ff Mon Sep 17 00:00:00 2001 From: Paddy Mullen Date: Fri, 18 Sep 2026 15:33:33 -0400 Subject: [PATCH 4/4] fix(styling): resolve column identity outside styling, fall back through the MRO (#966) Column identity (header_name / col_path) is now resolved by style_columns before styling runs, from orig_col_name or the df's rewrite map, so no styling failure can change a header and a missing orig_col_name no longer yields 'None'. style_column_with_fallback tries each style_column in the MRO, most specific first, so a failing subclass falls back to its parent's styling (e.g. DefaultMainStyling) instead of bare obj. Each attempt gets its own copy of col_meta. default_styling goes back to its original signature. Co-Authored-By: Claude Opus 5 --- buckaroo/dataflow/styling_core.py | 68 ++++++++++++++++++------------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/buckaroo/dataflow/styling_core.py b/buckaroo/dataflow/styling_core.py index e18f03f87..fb6273cb7 100644 --- a/buckaroo/dataflow/styling_core.py +++ b/buckaroo/dataflow/styling_core.py @@ -1,6 +1,6 @@ import copy import logging -from typing import Iterable, TypedDict, Union, List, Dict, Any, Literal, Optional +from typing import Iterable, TypedDict, Union, List, Dict, Any, Literal from typing_extensions import NotRequired, TypeAlias import pandas as pd @@ -434,10 +434,35 @@ def fix_column_config(cls, col: ColIdentifier, orig_col_name: ColIdentifier, bas summary_stats_key: str = 'all_stats' @classmethod - def default_styling(cls, col_name:Union[Iterable[str], str], orig_col_name:Optional[ColIdentifier]=None, /) -> ColumnConfig: - if orig_col_name is None: - orig_col_name = col_name - return cls.fix_column_config(col_name, orig_col_name, {'displayer_args': {'displayer': 'obj'}}) + def default_styling(cls, col_name:Union[Iterable[str], str], /) -> ColumnConfig: + return cls.fix_column_config(col_name, col_name, {'displayer_args': {'displayer': 'obj'}}) + + @classmethod + def style_column_with_fallback(cls, col:str, col_meta:ColMeta, orig_col_name:ColIdentifier) -> ColumnConfig: + """Try each style_column in the MRO, most specific first. + + A subclass that raises (or returns something that isn't a column + config) falls back to its parent's styling instead of bare obj, so a + bug in an extension only costs that column the extension's tweaks. + Every attempt gets its own copy of col_meta so edits made by a + failing style_column don't leak into the next attempt or the sd. + """ + for klass in cls.__mro__: + if 'style_column' not in klass.__dict__: + continue + style_column = klass.__dict__['style_column'].__get__(None, cls) + try: + return cls.fix_column_config(col, orig_col_name, style_column(col, dict(col_meta))) + except Exception as exc: + if len(col_meta) == 0 and len(cls.requires_summary) > 0: + # this is called in instantiation without col_meta, and that can cause failures + # we want to just swallow these errors and not warn + continue + # something unexpected happened here, warn so that the developer is notified + logger.warning(f"Warning, styling failed from {klass.__qualname__}.style_column (via {cls}) on column {col} with col_meta {col_meta}, falling back to the parent class") + logger.warning(exc) + # StylingAnalysis.style_column can't raise, so this is only reachable if it was patched out + return cls.fix_column_config(col, orig_col_name, {'displayer_args': {'displayer': 'obj'}}) @classmethod def get_dfviewer_config(cls, sd:SDType, df:pd.DataFrame) -> DFViewerConfig: @@ -459,30 +484,17 @@ def style_columns(cls, sd:SDType, df:pd.DataFrame) -> List[ColumnConfig]: skip_orig_cols.append(col) rewrites= dict( old_col_new_col(df)) + rewritten_to_orig = {v: k for k, v in rewrites.items()} for col, col_meta in sd.items(): - try: - orig_col_name = col_meta.get('orig_col_name') - if orig_col_name in skip_orig_cols or col_meta.get('merge_rule', None) == 'hidden': - - continue - #it actually gets tuples here - base_style: ColumnConfig = cls.fix_column_config(col, orig_col_name, cls.style_column(col, col_meta)) - except Exception as exc: - if len(col_meta) == 0 and len(cls.requires_summary) > 0: - # this is called in instantiation without col_meta, and that can cause failures - # we want to just swallow these errors and not warn - pass - else: - # something unexpected happened here, warn so that the develoepr is notified - logger.warning(f"Warning, styling failed from {cls} on column {col} with col_meta {col_meta} using default_styling instead") - logger.warning(exc) - # Always provide a style, not providing a style - # results in no display which is a very bad user - # experience - # keep the real header so a failure only costs this column its styling - base_style = cls.default_styling(col, col_meta.get('orig_col_name')) - - + if col_meta.get('orig_col_name') in skip_orig_cols or col_meta.get('merge_rule', None) == 'hidden': + continue + # the column's identity (header / col_path) is the framework's job, not the styling + # class's, so it's resolved outside of styling and survives any styling failure + orig_col_name = col_meta.get('orig_col_name') + if orig_col_name is None: + orig_col_name = rewritten_to_orig.get(col, col) + #it actually gets tuples here + base_style: ColumnConfig = cls.style_column_with_fallback(col, col_meta, orig_col_name) if 'column_config_override' in col_meta: #column_config_override, sent by the instantiation, gets set later