-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat: make plotly-express dataframe agnostic via narwhals #4790
Merged
+1,689
−834
Merged
Changes from 121 commits
Commits
Show all changes
131 commits
Select commit
Hold shift + click to select a range
9873e97
non core changes
FBruzzesi 0389591
_core overhaul
FBruzzesi ba93236
some _core fixes
FBruzzesi 421fc1d
tests replace sort_index(axis=1)
FBruzzesi ca5c820
reset_index in concat and allow any object to pandas
FBruzzesi a6aab24
trendline prep
FBruzzesi 7665f10
WIP Index
FBruzzesi ec4f250
clean from breakpoints
FBruzzesi 7e0d4c2
some tests fix
FBruzzesi 5543638
hotfix and tests output to pandas
FBruzzesi cd0dab7
FIX: columns never as index
FBruzzesi f334b32
getting there with the tests
FBruzzesi e5eb949
get_column instead of pandas slicing, unix to seconds
FBruzzesi 7747e30
bump narhwals, hierarchy fastpath
FBruzzesi ac00b36
fix to_unindexed_series
FBruzzesi da80c5b
fix trendline
FBruzzesi 8a72ba1
rm numpy dep in _core
FBruzzesi aeff203
fix: _check_dataframe_all_leaves
FBruzzesi 2041bef
(maybe) fix to_unindexed_series
FBruzzesi 71473f1
(maybe) fix to_unindexed_series
FBruzzesi 9f74c38
started tests with constructor
FBruzzesi 28587c9
added constructor to all tests
FBruzzesi 1bb2448
added some comments for fixme
FBruzzesi f45addf
to_py_scalar and more tests
FBruzzesi 5341759
dealing with exceptions and tests
FBruzzesi dfc957c
bump version, sort(...,nulls_last=True)
FBruzzesi 90f2667
We did it: no more dups in group by :D
FBruzzesi fb58d1b
concat_str
FBruzzesi ddb3b35
fix test_several_dataframes
FBruzzesi 37ce302
dedups customdata
FBruzzesi 4da8768
getting there
FBruzzesi 210e01a
xfail pyarrow chunked-array because name-less
FBruzzesi c00525e
all green with edge narhwals
FBruzzesi 3486a3e
add pandas nullable constructors in tests
FBruzzesi c0ce093
bump narwhals and address todos
FBruzzesi 0eb6951
check narwhals installation
FBruzzesi 844a6a9
rm unused comments
FBruzzesi 0c27789
rm unused code
FBruzzesi 0e6ff78
add pyarrow and narwhals to requirements_39_pandas_2_optional
FBruzzesi c2337c9
requirements, test requirements optional
FBruzzesi 2cc5d7b
refactor tests
FBruzzesi 1b27487
address feedbacks
FBruzzesi 23a23be
typos
FBruzzesi 7968cff
conftest
FBruzzesi cf76721
merge master
FBruzzesi 91db84b
mock interchange
FBruzzesi 5c6772e
optional requirements
FBruzzesi 9ec3f9e
move conftest in express folder
FBruzzesi 400a624
hotfix and figure_factory hexbin
FBruzzesi 1aa5163
old versions, polars[timezone], hotfix
FBruzzesi 594ded0
fix frame value in hexbin
FBruzzesi 6676061
copy numpy array
FBruzzesi d7d2884
hotfix hexbin mapbox
FBruzzesi d6ee676
Merge branch 'master' into plotly-with-narwhals
FBruzzesi 82c114d
fix test
FBruzzesi 0ceabc1
Merge branch 'plotly:master' into plotly-with-narwhals
FBruzzesi c9b626e
use lazy in process_dataframe_hierarchy
FBruzzesi 87841d1
fix custom sort in process_dataframe_pie
FBruzzesi ffa7b3b
Merge branch 'master' into plotly-with-narwhals
archmoj 3ba19ae
bump version and adjust core
FBruzzesi a70146b
use dtype.is_numeric
FBruzzesi 1fa9fe4
Merge branch 'master' into plotly-with-narwhals
FBruzzesi 0103aa6
revert test
FBruzzesi 673d141
Merge branch 'plotly-with-narwhals' of https://github.com/FBruzzesi/p…
FBruzzesi b858ed8
feedback adjustments
FBruzzesi bbcf438
Merge branch 'master' into plotly-with-narwhals
FBruzzesi 49efae2
raise if numpy is missing, conftest fix, typo
FBruzzesi a36bc24
__plotly_n_unique__
FBruzzesi c119153
Merge branch 'master' into plotly-with-narwhals
FBruzzesi 7416407
format
FBruzzesi 1867f6f
format
FBruzzesi d3a28c0
feedback adjustments
FBruzzesi e6e9994
use drop_null_keys, some pandas fastpaths
MarcoGorelli 64b8c70
bump narwhals version
MarcoGorelli 3f6b383
some improvements by Marco
FBruzzesi 755aea8
format and pyspark path
FBruzzesi 6f18021
add narwhals to requirements core
FBruzzesi 4d62e73
Update packages/python/plotly/plotly/express/_core.py
FBruzzesi a770fd8
refactor checking for df
MarcoGorelli 7d6f7d6
pushdown only for interchange libraries, sort out test
MarcoGorelli b8c10ec
Update packages/python/plotly/plotly/express/_core.py
MarcoGorelli 490b64a
fixup
MarcoGorelli f7fd4c9
Merge remote-tracking branch 'origin/plotly-with-narwhals' into plotl…
MarcoGorelli 8753acb
lint
MarcoGorelli 1429e6f
bump narwhals version
MarcoGorelli 878d4db
refactor checking for df and bump version
FBruzzesi 192e0a8
use token in process_dataframe_hierarchy
FBruzzesi de6761c
Range(label=...) for px.funnel
FBruzzesi bcfef68
improve error message and in-line comments
FBruzzesi 519cc68
better comments
FBruzzesi e5520a7
rm unused import and fix typo
FBruzzesi b855352
Merge branch 'master' into plotly-with-narwhals
FBruzzesi 51e2b23
make sure column + token is unique, replace **{} with .alias()
FBruzzesi 7ef9f28
WIP
FBruzzesi e9a367d
WIP
FBruzzesi 12fed31
Merge branch 'master' into plotly-with-narwhals
FBruzzesi 27b2996
use nw.get_native_namespace
FBruzzesi f27f959
Merge branch 'plotly-with-narwhals' of https://github.com/FBruzzesi/p…
FBruzzesi 126a79d
Merge branch 'master' into feat/dataframe-agnostic-data
FBruzzesi 7735366
add narwhals in various requirements
FBruzzesi b6516b4
docstrings
FBruzzesi 6f1389f
rm type hints, change post_agg to use alias
FBruzzesi db22268
feedback adjustments
FBruzzesi b514c01
move imports out, fix pyarrow
FBruzzesi ce8fb9a
rm unused narwhals wrapper
FBruzzesi e47827e
comment about stable api
FBruzzesi 9a9283a
update changelog
FBruzzesi 2630a5a
fixup time zone handling
MarcoGorelli fef6dbe
modin and cudf
FBruzzesi 48c7f62
defensive from_native call
FBruzzesi 18cc11c
typo
FBruzzesi d94cbf7
fixup timezones
FBruzzesi c320c46
move from object to datetime dtype in _plotly_utils/test/validators
FBruzzesi afdb31f
simplify ecdfnorm
MarcoGorelli 68ab52a
Merge pull request #4 from MarcoGorelli/ecdf-mode-perf
FBruzzesi b8ccec4
Merge branch 'master' into plotly-with-narwhals
FBruzzesi f102998
rm to_py_scalar call in for loop -> fix Pie performances
FBruzzesi 2df0427
Merge branch 'plotly-with-narwhals' of https://github.com/FBruzzesi/p…
FBruzzesi 55a0178
Merge branch 'master' into feat/dataframe-agnostic-data
FBruzzesi bb327d5
merge feat/dataframe-agnostic-data
FBruzzesi 7d611fb
use return_type directly when building datasets
FBruzzesi a22a7be
stocks date to string and test_trendline_on_timeseries fix
FBruzzesi 44a52e5
merge master and rm FIXME comment
FBruzzesi fc74b2e
do not repeat new_series unnecessarely
FBruzzesi 499e2fa
bump version, use numpy for range
FBruzzesi d2e1008
trigger ci now that new version is published
FBruzzesi 742b2ec
add narwhals to np2_optional.txt
FBruzzesi 269dea6
version
FBruzzesi b1dc48d
Merge branch 'master' into plotly-with-narwhals
MarcoGorelli 17fb96f
Merge branch 'master' into plotly-with-narwhals
FBruzzesi 9f2c55b
Merge branch 'master' into plotly-with-narwhals
FBruzzesi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 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 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 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
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.
Concerning stability and testing purposes, wondering if it's better to load specific version instead?
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.
Would you suggest to pin a specific narwhals version in the requirements?
We try to make some promises of non-breaking changes with the stable API. Additionally, we test each change against the main/master branches of the downstream libraries that adopted narwhals, in a github action.
Of course this is not perfect, but gives us some confidence when we make a change and a release
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.
hi @archmoj ! nice to meet you, and thanks for taking a look 🙏
I'd advise against pinning the Narwhals version exactly, and that's because it might conflict with other tools which are using Narwhals
For example, Altair currently has
narwhals>=1.5.2
, and this PR hasnarwhals>=1.12.0
. So, a user wanting to install both just needs to havenarwhals>=1.12.0
, and there's no issuesIf Plotly were to pin Narwhals exactly (say,
narwhals==1.12.0
), and Altair were to do the same (say,narwhals==1.10.0
), then users wouldn't be able to install both Altair and Plotly together, which would be a pityIndeed, as @FBruzzesi says, the intention behind
narwhals.stable.v1
is to have a stable and perfectly backwards-compatible API, in order to give downstream libraries (e.g. Plotly, Altair, Marimo, ...) the confidence to not pin the Narwhals version exactly, allowing for multiple major libraries to have Narwhals as a dependency without forbidding users to install them all togetherFurthermore, for major libraries, we run downstream tests in the Narwhals CI to check that any change we add won't break things for you
Happy to discuss further if you like of course (and to take a call if you'd like to discuss this or other topics) 🤗
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.
Thanks you very much for the clarification.
Please a comment including this information in this file so that we resolve it.
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.
Would the following comment under the import be enough?
It's a TL;DR of what Marco mentioned