Repository navigation
Fix Plotly Express mutating lists passed to x or y in wide mode - #5727
Merged
Merged
Conversation
args['wide_variable'] aliased the user-supplied x/y list, and process_args_into_dataframe replaces its elements with column-name strings in place. Copy it into a fresh list (this also accepts tuples, which previously failed on item assignment). Fixes plotly#4117
cpruijsen
force-pushed
the
fix/issue-4117
branch
from
September 15, 2026 15:05
efd34d4 to
1b6c477
Compare
camdecoster
requested changes
Oct 8, 2026
camdecoster
left a comment
camdecoster
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the fix! Once you update the test, I can approve this.
Comment on lines
+895
to
+905
| def test_wide_mode_does_not_mutate_x_or_y(): | ||
| # https://github.com/plotly/plotly.py/issues/4117 | ||
| df = pd.DataFrame(dict(a=[1, 2, 3], b=[4, 5, 6], c=[7, 8, 9])) | ||
| for arg in ["x", "y"]: | ||
| cols = ["a", "b"] | ||
| px.bar(df, **{arg: cols}) | ||
| assert cols == ["a", "b"] | ||
| cols = [0, 1] | ||
| df_int = pd.DataFrame([[1, 2], [3, 4]]) | ||
| px.histogram(df_int, x=cols) | ||
| assert cols == [0, 1] |
Contributor
There was a problem hiding this comment.
Could you update this test to check the case where the column names are integers to match the issue? With string names, the mutation doesn't do anything because "a" is the same as str("a").
| var_name = columns.name | ||
| if is_pd_like and isinstance(args["wide_variable"], native_namespace.Index): | ||
| args["wide_variable"] = list(args["wide_variable"]) | ||
| # copy into a new list so that the list provided by the user for |
Contributor
There was a problem hiding this comment.
Suggested change
| # copy into a new list so that the list provided by the user for | |
| # copy into a new list so that the object passed by the user for |
Contributor
Author
|
Thanks. The test now uses integer column names for both x and y, and it fails without the fix in _core.py. I also applied your comment wording. |
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.
In wide mode, Plotly Express mutates the list a caller passes as
xory, converting its values tostrings in place. The caller's own variable changes underneath them, which is surprising on its own
and breaks any later use of that list.
build_dataframeinplotly/express/_core.pyassigns the argument toargs["wide_variable"]andthen replaces entries in it. The copy that would have prevented this was conditional: it only ran when
the value was a pandas Index, so a plain list was aliased rather than copied and the replacement wrote
through to the caller's object.
The copy is now unconditional.
list()over a list is a shallow copy, which is all that is neededhere since only the entries are replaced, and the Index case takes the same path it did before.
The test asserts the caller's list is unchanged after the call, which fails on
main.Changelog entry included, per the repo convention.
Fixes #4117