gkneighb commented on PR #43573: URL: https://github.com/apache/superset/pull/43573#issuecomment-5882845084
@rusackas Done at `c47db173bb` — you were right that it's real rather than a style nag, and your trace through `merge_chart_form_data` matches what I measured. `sankey.py` now uses `model_dump(exclude_unset=True)` like `pie.py`, with a regression test beside the existing pie twin in `test_chart_utils.py` (`test_merge_sankey_preserves_omitted_defaults`). It runs the config through `DatasetValidator.normalize_column_names` first, exactly as `update_chart` does, then asserts an omitted `color_scheme`/`row_limit` survives while explicitly supplied values still win. It fails on the previous head and passes now. Since the same shape could be anywhere, I audited every registered plugin rather than just this one: | plugin | state | | --- | --- | | sankey, funnel, heatmap | had it — fixed on their own PRs | | radar | had it — fixed earlier on #43571 | | bubble | had it on master — **#44618** | | gantt | **not** affected — it captures `model_fields_set` before the dump and restores it after, which is equivalent | | the other 13 | already passed `exclude_unset=True` | So gantt looked guilty to a grep but is fine; I verified it by running `normalize_column_refs` and confirming `row_limit` stays out of `model_fields_set`. Thanks for tracing it instead of forwarding the bot thread — that's what made it obvious it was worth chasing across the family. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
