Skip to content

Fix unformatted placeholders in secondary_y and output_type error messages - #5772

Open
David-Wu1119 wants to merge 3 commits into
plotly:mainfrom
David-Wu1119:unformatted-error-placeholders
Open

David-Wu1119 wants to merge 3 commits into
plotly:mainfrom
David-Wu1119:unformatted-error-placeholders

Conversation

@David-Wu1119

@David-Wu1119 David-Wu1119 commented Sep 23, 2026 •

Copy link
Copy Markdown

Link to issue

No existing issue. I searched for both messages and found none.

Description of change

Two ValueError messages were never formatted, so users see the raw placeholders:

fig = make_subplots(rows=1, cols=2)
fig.add_scatter(x=[1], y=[1], row=1, col=2, secondary_y=True)
# ValueError:
# Subplot with type '{subplot_type}' at grid position ({row}, {col}) was not
# created with the secondary_y spec property set to True. ...

pio.from_json(fig.to_json(), output_type="Bogus")
# ValueError:
# Invalid output type: {output_type}

This PR makes both messages f-strings. After the change they read Subplot with type 'xy' at grid position (1, 2) was not ... and Invalid output type: Bogus.

Testing strategy

No new tests or changelog entry, per review. Checked by hand that both messages now read Subplot with type 'xy' at grid position (1, 2) was not ... and Invalid output type: Bogus. tests/test_core/test_subplots/ and tests/test_io/test_to_from_json.py pass (106 tests), and ruff format --check and ruff check with the pinned 0.11.12 pass on the changed files.

Guidelines

Found with a scan for string literals that contain {name} placeholders but are never formatted; written with AI assistance.

🤖 Generated with Claude Code

The error raised when adding a trace with `secondary_y=True` to a subplot
that was not created with the `secondary_y` spec, and the error raised for
an invalid `output_type` in `plotly.io.from_json`/`read_json`, were plain
strings that were never formatted, so users saw the literal
"{subplot_type}", "({row}, {col})" and "{output_type}". Format them like
the neighbouring messages in the same functions already do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 23, 2026 17:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@camdecoster camdecoster left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR! Could you please convert these to f-strings? Also, please remove the tests you added. They aren't needed. We also won't update the changelog for a change like this, so could you please revert that change?

@camdecoster camdecoster self-assigned this Sep 23, 2026
Per review: format the two messages with f-strings instead of
str.format(), and remove the added tests and CHANGELOG entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@David-Wu1119

David-Wu1119 commented Sep 24, 2026 •

Copy link
Copy Markdown
Author

Thanks @camdecoster! Done in dd7cdd4: switched both messages to f-strings and removed the tests and the changelog entry, so the diff is now just the two lines in plotly/_subplots.py and plotly/io/_utils.py.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants