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>
Optimize requires_js_object to avoid expensive trim_dict serialization
V.1.11.0 candidate
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 87.86% 87.82% -0.05%
==========================================
Files 210 210
Lines 31316 31522 +206
Branches 2413 2432 +19
==========================================
+ Hits 27516 27683 +167
- Misses 2826 2858 +32
- Partials 974 981 +7 ☔ 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
2 participants
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.
ENHANCEMENT: Align the API to Highcharts (JS) v.12.6. In particular, this includes:
Credits.eventsproperty.Boost.chunk_sizeproperty.Exporting.localproperty.Tooltip.show_delayandCrosshairOptions.show_delayproperties.Legend.max_widthsupport.headers,group_padding,node_size_by,traverse_to_leaf, andzoom_enabled.Tooltip.fixedandTooltip.positionsupport.TESTS: Added unit tests to confirm
Chart.module_urlsupport for local path.ENHANCEMENT: Updated dependencies and requirements to more-recent versions to address security patches.
ENHANCEMENT: Major performance optimization to data point serialization. (courtesy of @dcherman)
BUGFIX: Fixed import error associated with
requests.auth.HTTPBasicAuth. Closes BUG: HTTP basic authentication fails with "module 'requests' has no attribute 'HTTPBasicAuth'" #221