Add array_ok to SubplotidValidator, used in pie.legend #5465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

my-tien wants to merge 6 commits into plotly:main

from my-tien:legend-arrayok

Open

Add array_ok to SubplotidValidator, used in pie.legend #5465

my-tien wants to merge 6 commits into plotly:main from my-tien:legend-arrayok

+105 −21

Conversation

@my-tien

Copy link

@my-tien my-tien commented Jan 12, 2026 •

edited

Loading

Description

Update SubplotidValidator to handle arrays of values.

Additional fix for closing #5463.

Changes

Handle arrays in SubplotidValidator
Added array_ok tests

Testing

Be on main

Run the following code:

import plotly.graph_objects as go
pie = go.Pie(
 labels=["A", "B", "C"],
 values=[65, 15, 20],
 legend=["legend", "legend2"],
)
fig = go.Figure([pie], go.Layout())
fig.show()

Note that a ValueError is raised
Switch to this branch
Run the same code again
Note that chart is rendered correctly

Disclaimer

I am required to add that...

the software is provided "as is", without warranty of any kind, express or implied, including but not limited to the warranties of merchantability, fitness for a particular purpose and noninfringement. in no event shall the authors or copyright holders be liable for any claim, damages or other liability, whether in an action of contract, tort or otherwise, arising from, out of or in connection with the software or the use or other dealings in the software.

@my-tien


 Add array_ok to SubplotidValidator, used in pie.legend

1d665ba

@my-tien

Copy link

Author

my-tien commented Jan 12, 2026

FYI @camdecoster @emilykl

my-tien added 2 commits

January 12, 2026 14:34

@my-tien


 Fix names of test cases (forgot to rename them after copy-paste)

40532e4

@my-tien


 Run ruff format after adding support for arrays in subplotid validator.

3a50e45

camdecoster

camdecoster requested changes

Jan 12, 2026

View reviewed changes

Copy link

Contributor

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but there a few small changes to make. Thanks for the PR!

_plotly_utils/basevalidators.py Outdated Show resolved Hide resolved

_plotly_utils/basevalidators.py Show resolved Hide resolved

_plotly_utils/basevalidators.py Outdated

Comment on lines 1784 to 1800

def coerce(value, invalid_els):

if not isinstance(value, str):

invalid_els.append(value)

return value

match = fullmatch(self.regex, value)

if not match:

is_valid = False

invalid_els.append(value)

return value

else:

digit_str = match.group(1)

if len(digit_str) > 0 and int(digit_str) == 0:

is_valid = False

invalid_els.append(value)

return value

elif len(digit_str) > 0 and int(digit_str) == 1:

# Remove 1 suffix (e.g. x1 -> x)

v = self.base

is_valid = True

return self.base

else:

is_valid=True

returnvalue

Copy link

Contributor

@camdecoster camdecoster Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to refactor this function to just check if the given input is a valid id. Then you could reference that in the if/then block. It feels weird to define invalid_els for the single value case.

Copy link

Author

@my-tien my-tien Jan 13, 2026 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it like this because the return value is already used for the coercion result. (e.g. legend1 → legend). I changed it to return a tuple (coercion result, success) now, maybe this is clearer? Let me know what you think.

While doing that, I noticed that the coercion wasn't actually tested. I added a test for this as well.

my-tien added 3 commits

January 13, 2026 11:19

@my-tien


 Fix docstring and description of SubplotidValidator

d1993cc

- added missing arrayOk in docstring
- removed replaced description
- Fixed indentation

@my-tien


 changed helper function in SubplotidValidator

b809b46

- return success instead of writing into passed array

@my-tien


 Separate test for coercion of geo1 to geo and legend1 to legend

cef18a9

@my-tien my-tien force-pushed the legend-arrayok branch from 2461bc7 to cef18a9 Compare

January 13, 2026 10:44

Labels

None yet

2 participants

@my-tien @camdecoster

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add array_ok to SubplotidValidator, used in pie.legend #5465

Are you sure you want to change the base?

Add array_ok to SubplotidValidator, used in pie.legend #5465

Conversation

@my-tien my-tien commented Jan 12, 2026 •

edited

Loading

Uh oh!

Description

Changes

Testing

Uh oh!

my-tien commented Jan 12, 2026

Uh oh!

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

@camdecoster camdecoster Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

@my-tien my-tien Jan 13, 2026 •

edited

Loading

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Add array_ok to SubplotidValidator, used in pie.legend #5465

Are you sure you want to change the base?

Add array_ok to SubplotidValidator, used in pie.legend #5465

Conversation

@my-tien my-tien commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Testing

Uh oh!

my-tien commented Jan 12, 2026

Uh oh!

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

@camdecoster camdecoster Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

@my-tien my-tien Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

@my-tien my-tien commented Jan 12, 2026 •

edited

Loading

@my-tien my-tien Jan 13, 2026 •

edited

Loading