feat(config): add EXTRA_PANDAS_POSTPROCESSING_OPS extension point - #43337
Conversation
|
The update to superset/models/helpers.py |
dosubot
Bot
added
change:backend
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #43337 +/- ##
==========================================
- Coverage 66.73% 66.73% -0.01%
==========================================
Files 2876 2876
Lines 164228 164256 +28
Branches 37891 37896 +5
==========================================
+ Hits 109603 109616 +13
- Misses 52466 52479 +13
- Partials 2159 2161 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
villebro
left a comment
villebro
left a comment
There was a problem hiding this comment.
Looks good - One minor nit. Also, could we add a test that patches the config with custom op and validate that it mutates the result expectedly?
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Code Review Agent Run #7989d1Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
A couple of small things worth cleaning up sometime, neither blocking: the |
|
Good catch by codeant on the |
rusackas
added
the
merge-if-green
Made both the changes - they were quick and valid enough to come through together in this PR. Thanks for pointing these out! |
Code Review Agent Run #af7023Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
|
||
| extra_op_names = list( | ||
| pandas_postprocessing.build_extra_ops_map( | ||
| current_app.config.get("EXTRA_PANDAS_POSTPROCESSING_OPS", []) |
There was a problem hiding this comment.
Validating even built-in operations now dereferences current_app, so callers that load chart query schemas outside an app context fail with RuntimeError where they previously succeeded. Could this only read the extra-op config when needed, or handle the missing context like the neighboring config-backed validators?
There was a problem hiding this comment.
Addressed. Two changes:
- Built-ins short-circuit before any config access - If value in self._builtin_ops: return. Validating a built-in operation never dereferences current_app, so the previously-working no-app-context path is restored.
- Missing context is handled like the neighboring validators. The config read is wrapped in try/except RuntimeError, matching the existing get_time_grain_choices and get_max_prophet_periods pattern in the same file. Outside a context the extra-op list is treated as empty, so an unknown operation still produces a clean ValidationError rather than a RuntimeError.
One Caveat - The error message outside an app context now lists only built-ins. That's the same message the code produced before this PR, so it's not a regression, but it does mean a spec-generation context won't advertise operator-registered ops.
| @@ -545,12 +546,19 @@ def exec_post_processing(self, df: DataFrame) -> DataFrame: | |||
| _("`operation` property of post processing object undefined") | |||
| ) | |||
| if not hasattr(pandas_postprocessing, operation): | |||
There was a problem hiding this comment.
An extra callable named build_extra_ops_map passes validation and the startup collision check, but hasattr treats the helper as built-in and invokes it instead of the configured callable. Could this dispatch only names in __all__ before looking in extra_ops?
There was a problem hiding this comment.
Six names are reachable via hasattr but not in __all__ - Dispatch now gates on __all__ instead of hasattr, so it matches what the schema validates against and what the startup collision check warns about.
The new tests catch the bug by stashing the fix, all 6 parametrized cases fail on the old hasattr dispatch and pass with the fix in.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #33e5d4Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
4 participants
SUMMARY
Following the removal of viz.py in #41750, Superset's migration from legacy v0 charts to the modern query context API (v1) removes the ability to inject arbitrary Python dataframe transformations that some custom charts previously relied on. Charts performing custom post-query data manipulation in Python had no equivalent hook in the v1 pipeline.
This PR introduces EXTRA_PANDAS_POSTPROCESSING_OPS, a config-level extension point that allows operators to register custom post-processing functions alongside Superset's built-in pandas post-processing operations. A chart's post_processing query context can reference these by name, and the marshmallow schema validator is updated to accept them at validation time.
How it works
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION