Use plotly.js base64 API to store and pass typed arrays declared by numpy, pandas, etc.
#4470
Conversation
Co-authored-by: Liam Connors <liam@plot.ly>
- fixed Conflicts in packages/python/plotly/plotly/tests/test_optional/test_figure_factory/test_figure_factory.py packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py
Convert base64 in BaseFigure.to_dict instead of validate_coerce
|
@marthacryan Please resolve the conflicts with master; then merge master into this branch. |
|
thank you everyone |
- Removes a speed comparison that is no longer needed because the base64 encoding makes it faster - Updates a test to use numpy arrays to account for plotly express automatically converting numerical lists to pandas arrays. - Remove unnecessary enumerate
| @@ -372,38 +372,6 @@ def test_invalid_encode_exception(self): | |||
| with self.assertRaises(TypeError): | |||
| _json.dumps({"a": {1}}, cls=utils.PlotlyJSONEncoder) | |||
|
|
|||
| def test_fast_track_finite_arrays(self): | |||
There was a problem hiding this comment.
@marthacryan
Could we revert this change now or we should drop the test?
There was a problem hiding this comment.
I removed this because it was failing. Presumably it's just faster now?
test/percy/compare-pandas.py
Outdated
| if any(l1 != l2 for l1, l2 in zip(fig1, fig2)): | ||
| print("".join(difflib.unified_diff(fig1, fig2))) | ||
| raise ValueError(f"Pandas 1/2 difference in {filename}") | ||
| if filename not in [ |
There was a problem hiding this comment.
@marthacryan / @archmoj What is the reason for excluding these particular files?
| are skipped for conversion to the typed array spec | ||
| """ | ||
| skipped_keys = ["geojson", "layer", "range"] | ||
| return any(skipped_key in key for skipped_key in skipped_keys) |
There was a problem hiding this comment.
@marthacryan Shouldn't the test be skipped_key == key rather than skipped_key in key ?
Or we really do want to be checking that the skipped key is a substring of key?
There was a problem hiding this comment.
Also I think the docstring comment is no longer accurate
| v = copy_to_readonly_numpy_array(v) | ||
|
|
||
| np = get_module("numpy", should_load=False) | ||
| if not isinstance(v, np.ndarray): |
There was a problem hiding this comment.
Won't this line raise an error if numpy is not installed? (Since get_module will return None in that case.)
Is that ok?
This PR make use of this plotly.js PR and changes the default behavior to pass numpy arrays as plotly.js base64 spec.
Code Changes
to_typed_array_specfunction to use for converting numpy arrays to the "typed array spec" type that plotly.js expects when receiving base64 encoded arrays.validate_coercefunction of theDataArrayValidatorclass to base64 encode numpy arrays and store them in the "typed array spec" format (by callingto_typed_array_spec)