From a457f0ee5acda7d10a91fa7d1a2fac725bd06be4 Mon Sep 17 00:00:00 2001 From: Guy Rosin Date: Sun, 13 Aug 2023 16:52:27 +0300 Subject: [PATCH 1/5] Consider necessary columns from complex arguments when interchanging dataframes --- .../python/plotly/plotly/express/_core.py | 10 ++++++- .../test_optional/test_px/test_px_input.py | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/python/plotly/plotly/express/_core.py b/packages/python/plotly/plotly/express/_core.py index b889bec88df..77d9226b138 100644 --- a/packages/python/plotly/plotly/express/_core.py +++ b/packages/python/plotly/plotly/express/_core.py @@ -1419,9 +1419,17 @@ def build_dataframe(args, constructor): else: # Save precious resources by only interchanging columns that are # actually going to be plotted. - columns = [ + necessary_columns = [ i for i in args.values() if isinstance(i, str) and i in columns ] + for field in args: + if field in array_attrables and isinstance( + args[field], (list, dict) + ): + necessary_columns.extend( + [i for i in args[field] if i in columns] + ) + columns = list(dict.fromkeys(necessary_columns)) args["data_frame"] = pd.api.interchange.from_dataframe( args["data_frame"].select_columns_by_name(columns) ) diff --git a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py index fa0f1298fdc..0f2b0668bf6 100644 --- a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py +++ b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py @@ -327,6 +327,32 @@ def test_build_df_from_vaex_and_polars(test_lib): ) +@pytest.mark.skipif( + version.parse(pd.__version__) < version.parse("2.0.2"), + reason="plotly doesn't use a dataframe interchange protocol for pandas < 2.0.2", +) +@pytest.mark.parametrize("test_lib", ["vaex", "polars"]) +def test_build_df_with_hover_data_from_vaex_and_polars(test_lib): + if test_lib == "vaex": + import vaex as lib + else: + import polars as lib + + # take out the 'species' columns since the vaex implementation does not cover strings yet + iris_pandas = px.data.iris()[["petal_width", "sepal_length", "sepal_width"]] + iris_vaex = lib.from_pandas(iris_pandas) + args = dict( + data_frame=iris_vaex, + x="petal_width", + y="sepal_length", + hover_data=["sepal_width"], + ) + out = build_dataframe(args, go.Scatter) + assert_frame_equal( + iris_pandas.reset_index()[out["data_frame"].columns], out["data_frame"] + ) + + def test_timezones(): df = pd.DataFrame({"date": ["2015-04-04 19:31:30+1:00"], "value": [3]}) df["date"] = pd.to_datetime(df["date"]) From e2a7cb4d97c6ee4086cd5984cbf8a7b71837b8a4 Mon Sep 17 00:00:00 2001 From: Guy Rosin Date: Tue, 15 Aug 2023 01:03:09 +0300 Subject: [PATCH 2/5] No type check for array_attrables, add test for a hover_data dictionary --- packages/python/plotly/plotly/express/_core.py | 4 +--- .../plotly/tests/test_optional/test_px/test_px_input.py | 7 +++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/python/plotly/plotly/express/_core.py b/packages/python/plotly/plotly/express/_core.py index 77d9226b138..274339e58dc 100644 --- a/packages/python/plotly/plotly/express/_core.py +++ b/packages/python/plotly/plotly/express/_core.py @@ -1423,9 +1423,7 @@ def build_dataframe(args, constructor): i for i in args.values() if isinstance(i, str) and i in columns ] for field in args: - if field in array_attrables and isinstance( - args[field], (list, dict) - ): + if args[field] is not None and field in array_attrables: necessary_columns.extend( [i for i in args[field] if i in columns] ) diff --git a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py index 0f2b0668bf6..fcb9cf91018 100644 --- a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py +++ b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py @@ -332,7 +332,10 @@ def test_build_df_from_vaex_and_polars(test_lib): reason="plotly doesn't use a dataframe interchange protocol for pandas < 2.0.2", ) @pytest.mark.parametrize("test_lib", ["vaex", "polars"]) -def test_build_df_with_hover_data_from_vaex_and_polars(test_lib): +@pytest.mark.parametrize( + "hover_data", [["sepal_width"], {"sepal_length": False, "sepal_width": ":.2f"}] +) +def test_build_df_with_hover_data_from_vaex_and_polars(test_lib, hover_data): if test_lib == "vaex": import vaex as lib else: @@ -345,7 +348,7 @@ def test_build_df_with_hover_data_from_vaex_and_polars(test_lib): data_frame=iris_vaex, x="petal_width", y="sepal_length", - hover_data=["sepal_width"], + hover_data=hover_data, ) out = build_dataframe(args, go.Scatter) assert_frame_equal( From 83c2260f1d7e26feace3c911dcbc0f6cf245031a Mon Sep 17 00:00:00 2001 From: Guy Rosin Date: Tue, 15 Aug 2023 10:37:22 +0300 Subject: [PATCH 3/5] Necessary columns can be unordered --- packages/python/plotly/plotly/express/_core.py | 10 ++++------ .../tests/test_optional/test_px/test_px_input.py | 8 +++++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/python/plotly/plotly/express/_core.py b/packages/python/plotly/plotly/express/_core.py index 274339e58dc..275efaf3b4a 100644 --- a/packages/python/plotly/plotly/express/_core.py +++ b/packages/python/plotly/plotly/express/_core.py @@ -1419,15 +1419,13 @@ def build_dataframe(args, constructor): else: # Save precious resources by only interchanging columns that are # actually going to be plotted. - necessary_columns = [ + necessary_columns = { i for i in args.values() if isinstance(i, str) and i in columns - ] + } for field in args: if args[field] is not None and field in array_attrables: - necessary_columns.extend( - [i for i in args[field] if i in columns] - ) - columns = list(dict.fromkeys(necessary_columns)) + necessary_columns.update(i for i in args[field] if i in columns) + columns = list(necessary_columns) args["data_frame"] = pd.api.interchange.from_dataframe( args["data_frame"].select_columns_by_name(columns) ) diff --git a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py index fcb9cf91018..a11fe276e34 100644 --- a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py +++ b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py @@ -8,6 +8,7 @@ from plotly.express._core import build_dataframe from pandas.testing import assert_frame_equal + # Fixtures # -------- @pytest.fixture @@ -292,9 +293,10 @@ def __dataframe__(self): ) as mock_from_dataframe: build_dataframe(args, go.Scatter) mock_from_dataframe.assert_called_once_with(interchange_dataframe_reduced) - interchange_dataframe.select_columns_by_name.assert_called_with( - ["petal_width", "sepal_length"] - ) + assert set(interchange_dataframe.select_columns_by_name.call_args.args[0]) == { + "petal_width", + "sepal_length", + } args = dict(data_frame=input_dataframe_reduced, color=None) with mock.patch( From f251f0825031a6c1960ada7308e0f1df96920e70 Mon Sep 17 00:00:00 2001 From: Guy Rosin Date: Tue, 15 Aug 2023 11:03:25 +0300 Subject: [PATCH 4/5] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 298927812ee..aec6b6c1e7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## UNRELEASED + +### Fixed +- Fixed issue with necessary columns from complex arguments dropped when interchanging dataframes [[#4324](https://github.com/plotly/plotly.py/pull/4324)] ## [5.16.0] - 2023-08-11 From 1625d35aa0c7cca227ebcf0bf9ead746165820d5 Mon Sep 17 00:00:00 2001 From: Guy Rosin Date: Tue, 15 Aug 2023 11:04:06 +0300 Subject: [PATCH 5/5] Support python<3.8 --- .../plotly/plotly/tests/test_optional/test_px/test_px_input.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py index a11fe276e34..7c9ad6ac1ea 100644 --- a/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py +++ b/packages/python/plotly/plotly/tests/test_optional/test_px/test_px_input.py @@ -293,7 +293,7 @@ def __dataframe__(self): ) as mock_from_dataframe: build_dataframe(args, go.Scatter) mock_from_dataframe.assert_called_once_with(interchange_dataframe_reduced) - assert set(interchange_dataframe.select_columns_by_name.call_args.args[0]) == { + assert set(interchange_dataframe.select_columns_by_name.call_args[0][0]) == { "petal_width", "sepal_length", }