Repository navigation
Fix Image zmin/zmax rejecting values matching the colormodel - #5869
Open
DresdenGman wants to merge 1 commit into
Open
DresdenGman wants to merge 1 commit into
DresdenGman wants to merge 1 commit into
Conversation
The image trace's zmin/zmax validators required exactly 4 elements, but the number of color components depends on the colormodel: 3 for rgb/hsl and 4 for rgba/rgba256/hsla. Validators cannot see the colormodel of the trace being validated, so mark zmin/zmax as freeLength in codegen's preprocess_schema (accepting up to 4 elements) and regenerate the validators and graph objects. Closes plotly#1973
This branch has not been deployed
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.
Link to issue
Closes #1973
Description of change
go.Imagerejectedzmin/zmaxvalues whose length matches the trace'scolormodel: with
colormodel='rgb'the valid 3-element bounds raised aValueErrorbecause the validator required exactly 4 elements (the rgbalength). The number of components is 3 for
rgb/hsland 4 forrgba/rgba256/hsla— the attributes' own descriptions say so — butvalidators are stateless and cannot see the
colormodelof the trace beingvalidated, so the fixed 4-element length from the plotly.js schema cannot be
satisfied per-colormodel.
This marks
zmin/zmaxof the image trace asfreeLengthinpreprocess_schemaincodegen/__init__.py— the central place for schemaadjustments before code generation — so they accept up to 4 elements. This
uses the existing info-array option the schema already applies to e.g.
updatemenu button
argsand parcoordsconstraintrange; no validator codechanges were needed. The regenerated
plotly/validators/_validators.jsongains
"free_length": trueon exactly these two properties, and thegenerated docstrings in
plotly/graph_objs/_image.pynow read "a list ortuple of up to 4 elements".
One trade-off to be explicit about:
freeLengthaccepts any length up to 4,so 1- and 2-element values (and an empty list) are now also accepted, as they
already are for the other
freeLengthproperties. Every element is stillvalidated as a number and lengths above 4 are still rejected. Restricting to
exactly 3 or 4 would need a new schema/validator option that plotly.js does
not define; if the schema in plotly.js itself gains
freeLengthfor theseattributes, the preprocessing step here can be dropped.
Demo
Not applicable — validation-only change with no visual output difference
(the figure above previously raised instead of rendering).
Testing strategy
Added
tests/test_core/test_graph_objs/test_image.py(16 tests): the #1973example verbatim, matching-length bounds for all five colormodels, a
4-element value for a 3-component model (the previously accepted workaround
keeps working), the default colormodel, independence from property
assignment order, 5-element values rejected for every colormodel,
non-numeric elements rejected, and
Heatmapscalarzmin/zmaxas anunaffected control. The 5 tests covering 3-element values fail on main and
pass with this change.
Full runs of
tests/test_plotly_utils/validators/andtests/test_core/test_graph_objs/: 1555 passed / 2 failed on main vs.1571 passed / 2 failed with this change — the delta is exactly the 16 new
tests, and both failures are pre-existing environment issues
(
test_deepcopy_dataframe[polars]/[pyarrow], missing optionaldependencies), identical before and after.
Additional information (optional)
The fix is applied at the code-generation layer rather than by editing
generated files:
preprocess_schemaalready hosts schema adjustments ofthis kind (e.g. the sankey
concentrationscalesrename). I verified thatre-running code generation on unmodified main reproduces the checked-in
_validators.jsonexactly, so the regenerated diff contains only the twointended entries.
Guidelines