Optimize requires_js_object to avoid expensive trim_dict serialization#222
Open
dcherman wants to merge 1 commit intohighcharts-for-python:masterfrom
Open
Optimize requires_js_object to avoid expensive trim_dict serialization#222dcherman wants to merge 1 commit intohighcharts-for-python:masterfrom
dcherman wants to merge 1 commit intohighcharts-for-python:masterfrom
Conversation
The `requires_js_object` property on data points was calling
`self.to_dict()` + `self.trim_dict()` — a full serialization round-trip —
just to determine whether a point could be represented as a JS array
(e.g. `[1, 2]`) or needed a JS object (e.g. `{x: 1, y: 2, color: "red"}`).
For large data series this was extremely slow. With ~7,500 points,
`display()` took ~17 seconds, with the vast majority spent inside
`trim_dict` performing type checks via the `validator_collection` library.
Each validator/checker call in that library is wrapped by a decorator
(`disable_on_env` / `disable_checker_on_env`) that calls `os.getenv()` on
every invocation to check whether it should be disabled — resulting in
~6 million `os.getenv()` calls per render.
The upstream `validator_collection` library
(https://github.com/insightindustry/validator-collection) has not been
updated in several years, so rather than wait for a fix there, this change
avoids triggering the expensive code path entirely.
The fix replaces the `to_dict()` + `trim_dict()` approach with a direct
inspection of `_to_untrimmed_dict()`, checking whether any non-array
properties hold non-None, non-empty values. This preserves the same
semantics while bypassing all validator overhead.
Result: `display()` drops from ~17.3s to ~0.96s (18x speedup) for a chart
with 7,500 data points.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #222 +/- ##
==========================================
+ Coverage 87.86% 87.88% +0.01%
==========================================
Files 210 210
Lines 31316 31322 +6
Branches 2413 2416 +3
==========================================
+ Hits 27516 27526 +10
+ Misses 2826 2824 -2
+ Partials 974 972 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
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.
The
requires_js_objectproperty on data points was callingself.to_dict()+self.trim_dict()— a full serialization round-trip — just to determine whether a point could be represented as a JS array (e.g.[1, 2]) or needed a JS object (e.g.{x: 1, y: 2, color: "red"}).For large data series this was extremely slow. With ~7,500 points,
display()took ~17 seconds, with the vast majority spent insidetrim_dictperforming type checks via thevalidator_collectionlibrary. Each validator/checker call in that library is wrapped by a decorator (disable_on_env/disable_checker_on_env) that callsos.getenv()on every invocation to check whether it should be disabled — resulting in ~6 millionos.getenv()calls per render.The upstream
validator_collectionlibrary(https://github.com/insightindustry/validator-collection) has not been updated in several years, so rather than wait for a fix there, this change avoids triggering the expensive code path entirely.
The fix replaces the
to_dict()+trim_dict()approach with a direct inspection of_to_untrimmed_dict(), checking whether any non-array properties hold non-None, non-empty values. This preserves the same semantics while bypassing all validator overhead.Result:
display()drops from ~17.3s to ~0.96s (18x speedup) for a chart with 7,500 data points.