fix(styling): keep column identity outside styling, fall back to parent styling on failure (#966) - #968
Open
paddymul wants to merge 5 commits into
Open
fix(styling): keep column identity outside styling, fall back to parent styling on failure (#966)#968paddymul wants to merge 5 commits into
paddymul wants to merge 5 commits into
Conversation
#966) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8f23eff9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+423
to
+425
| assert dfvc['column_config'] == [ | ||
| {'col_name': 'a', 'header_name': 'foo', 'displayer_args': {'displayer': 'obj'}}, | ||
| {'col_name': 'b', 'header_name': 'bar', 'displayer_args': {'displayer': 'obj'}}] |
There was a problem hiding this comment.
When any style_column implementation raises, StylingAnalysis.style_columns still falls back via default_styling(col), which uses the rewritten IDs (a, b) as the header/original path. This commit changes only the tests, so these assertions (and the multi-index case below) fail while the advertised fix is not present; update the fallback to pass col_meta['orig_col_name'] when available.
Useful? React with 👍 / 👎.
Contributor
📦 TestPyPI package publishedpip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.15.6.dev35388730737or with uv: uv pip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.15.6.dev35388730737MCP server for Claude Codeclaude mcp add buckaroo-table -- uvx --from "buckaroo[mcp]==0.15.6.dev35388730737" --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo-table📖 Docs preview🎨 Storybook preview |
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 <noreply@anthropic.com>
…hout orig_col_name (#966) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ugh 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 <noreply@anthropic.com>
paddymul
changed the title
…header-name # Conflicts: # buckaroo/dataflow/styling_core.py
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #966.
When
style_columnraised,style_columnsfell back todefault_styling(col), which passed the rewritten id as bothcolandorig_col_name. Every failed column showeda,b, ... as its header. A styling subclass that raised on every column made the table look like it had lost its column names.Two changes:
Column identity is resolved outside styling.
style_columnsworks out each column's header (orcol_path) before any styling runs. It usescol_meta['orig_col_name'], falling back to the df's rewrite map, so no styling failure can change a header. A column with noorig_col_name, as in the instantiation pass, also no longer getsheader_name: 'None'when styling succeeds.A failing style_column falls back through the MRO.
style_column_with_fallbacktries eachstyle_columnin the MRO, most specific first. A subclass ofDefaultMainStylingthat raises, or returns something that isn't a column config, getsDefaultMainStyling's styling for that column instead of bareobj. An intermediate class likeDiffStylingis kept too. A class that replacesStylingAnalysisdirectly instead of extendingDefaultMainStylingonly hasobjto fall back on. Each attempt gets a shallow copy ofcol_meta, so a failingstyle_columnthat edits it doesn't affect the next attempt or the sd.Each failure still logs a warning naming the class that raised. #971 tracks putting the error into the column config so it's visible without server logs. That matters more now, because a column that fell back to its parent looks almost right.
Commits:
f8f23effandcf519e13add failing tests, which CI showed failing.d4bd8c00was a first, narrower fix.e3af3083replaces it.🤖 Generated with Claude Code