feat: make plotly-express dataframe agnostic via narwhals #4790
refactor tests
- Loading branch information
commit 2cc5d7b701a1d0090bac0efcf96f1417f1778a94
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,22 +2,10 @@ | |
| import plotly.io as pio | ||
| import narwhals.stable.v1 as nw | ||
| import numpy as np | ||
| import pytest | ||
| from itertools import permutations | ||
|
|
||
|
|
||
| def test_scatter(constructor): | ||
|
There was a problem hiding this comment. @FBruzzesi I'm not seeing the benefit to calling There was a problem hiding this comment. I think it's so that in L14 There was a problem hiding this comment. Got it, I see it now, thanks. I see a few tests further down, though, where it doesn't seem necessary -- what about the following?
There was a problem hiding this comment. I am addressing those, thanks for spotting them! Replacing all with regex was definitly a mistake 🙈 Edit: |
||
| data = px.data.iris().to_dict(orient="list") | ||
| iris = nw.from_native(constructor(data)) | ||
|
|
@@ -29,7 +17,6 @@ def test_scatter(constructor): | |
| assert fig.data[0].mode == "markers" | ||
|
|
||
|
|
||
| def test_custom_data_scatter(constructor): | ||
| data = px.data.iris().to_dict(orient="list") | ||
| iris = nw.from_native(constructor(data)) | ||
|
|
@@ -80,7 +67,6 @@ def test_custom_data_scatter(constructor): | |
| ) | ||
|
|
||
|
|
||
| def test_labels(constructor): | ||
| data = px.data.tips().to_dict(orient="list") | ||
| tips = nw.from_native(constructor(data)) | ||
|
|
@@ -106,7 +92,6 @@ def test_labels(constructor): | |
| assert fig.layout.annotations[4].text.startswith("TIME") | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ["extra_kwargs", "expected_mode"], | < 10000 /tr>||
| [ | ||
|
|
@@ -129,7 +114,6 @@ def test_line_mode(constructor, extra_kwargs, expected_mode): | |
| assert fig.data[0].mode == expected_mode | ||
|
|
||
|
|
||
| def test_px_templates(constructor): | ||
| try: | ||
| import plotly.graph_objects as go | ||
|
|
@@ -300,7 +284,6 @@ def assert_orderings(constructor, days_order, days_check, times_order, times_che | |
| assert trace.marker.color == color_sequence[i] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("days", permutations(["Sun", "Sat", "Fri", "x"])) | ||
| @pytest.mark.parametrize("times", permutations(["Lunch", "x"])) | ||
| def test_orthogonal_and_missing_orderings(constructor, days, times): | ||
|
|
@@ -309,7 +292,6 @@ def test_orthogonal_and_missing_orderings(constructor, days, times): | |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("days", permutations(["Sun", "Sat", "Fri", "Thur"])) | ||
| @pytest.mark.parametrize("times", permutations(["Lunch", "Dinner"])) | ||
| def test_orthogonal_orderings(constructor, days, times): | ||
|
|
@@ -322,7 +304,6 @@ def test_permissive_defaults(): | |
| px.defaults.should_not_work = "test" | ||
|
|
||
|
|
||
| def test_marginal_ranges(constructor): | ||
| data = px.data.tips().to_dict(orient="list") | ||
| df = nw.from_native(constructor(data)) | ||
|
|
@@ -339,7 +320,6 @@ def test_marginal_ranges(constructor): | |
| assert fig.layout.yaxis3.range is None | ||
|
|
||
|
|
||
| def test_render_mode(constructor): | ||
| data = px.data.gapminder().to_dict(orient="list") | ||
| df = nw.from_native(constructor(data)) | ||
|
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,22 +2,10 @@ | |
| import narwhals.stable.v1 as nw | ||
| import numpy as np | ||
| import pandas as pd | ||
| import pytest | ||
| from collections import OrderedDict # an OrderedDict is needed for Python 2 | ||
|
|
||
|
|
||
| def test_skip_hover(constructor): | ||
| data = px.data.iris().to_dict(orient="list") | ||
| df = constructor(data) | ||
|
|
@@ -31,7 +19,6 @@ def test_skip_hover(constructor): | |
| assert fig.data[0].hovertemplate == "species_id=%{marker.size}<extra></extra>" | ||
|
|
||
|
|
||
| def test_hover_data_string_column(constructor): | ||
| data = px.data.tips().to_dict(orient="list") | ||
| df = constructor(data) | ||
|
|
@@ -44,7 +31,6 @@ def test_hover_data_string_column(constructor): | |
| assert "sex" in fig.data[0].hovertemplate | ||
|
|
||
|
|
||
| def test_composite_hover(constructor): | ||
| data = px.data.tips().to_dict(orient="list") | ||
| df = constructor(data) | ||
|
|
@@ -105,7 +91,6 @@ def test_newdatain_hover_data(): | |
| ) | ||
|
|
||
|
|
||
| 104E8 def test_formatted_hover_and_labels(constructor): | ||
| data = px.data.tips().to_dict(orient="list") | ||
| df = constructor(data) | ||
|
|
@@ -191,7 +176,6 @@ def test_fail_wrong_column(): | |
| ) | ||
|
|
||
|
|
||
| def test_sunburst_hoverdict_color(constructor): | ||
| data = px.data.gapminder().query("year == 2007").to_dict(orient="list") | ||
| df = constructor(data) | ||
|
|
@@ -205,10 +189,10 @@ def test_sunburst_hoverdict_color(constructor): | |
| assert "color" in fig.data[0].hovertemplate | ||
|
|
||
|
|
||
| def test_date_in_hover(request, constructor): | ||
|
There was a problem hiding this comment. @FBruzzesi What does this mean in practice, in terms of how we should document this limitation in the docs? Something like "Datetimes converted to strings for non-Pandas input dataframes will lose their timezone information"? There was a problem hiding this comment. ooh this is a really good point, thanks! I just tried this out, and indeed, there is a difference:
@FBruzzesi here's an example to test this with: df = pl.DataFrame({'price': [40, 50], 'datetime': [datetime(2020,1,1), datetime(2020,1,2)]})
df = df.with_columns(pl.col('datetime').dt.replace_time_zone('Asia/Kathmandu'))
px.scatter(df, x='datetime', y='price')then, for Polars, we see:
whereas for pandas:
I'll have to check the logic, but the way to get the local datetime in Narwhals would be |
||
| if "pyarrow_table" in str(constructor) or "polars_eager" in str(constructor): | ||
| request.applymarker(pytest.mark.xfail) | ||
|
|
||
| df = nw.from_native( | ||
| constructor({"date": ["2015-04-04 19:31:30+01:00"], "value": [3]}) | ||
| ).with_columns(date=nw.col("date").str.to_datetime(format="%Y-%m-%d %H:%M:%S%z")) | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the constructor functions were modified to accept either a Pandas df or a dict? Then if a Pandas df is passed it could call
.to_dict()on it. Would save a bit of boilerplate in the tests.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it makes sense. However let me mention that after our meeting today I started working to allow data(set) module to be dataframe agnostic. Different PR should be ready tomorrow and it may be a quick win to simplify data loading in tests as well. I will ping you there