Skip to content
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

FIX-#398: Add fix for pandas 1.3 #428

Merged
merged 6 commits into from
Nov 3, 2021

Conversation

westernguy2
Copy link
Contributor

Signed-off-by: Kunal Agarwal [email protected]

Overview

Pandas 1.3.x changes the file structure of pd.io.parsers. As a result, in order to read any data in as a LuxDataFrame, the following change must be made.

Changes

I've changed __init__ to account for the new file structure mentioned above. I've also updated the requirements to require pandas>= 1.3.0 since the change will cause Lux to break on any Pandas version below 1.3.0.

Currently 3 tests are failing due to a different update in Pandas 1.3.x which causing history to not propagate in some instances.

Example Output

The example given in #398 displays the Lux widget as expected now.

@codecov
Copy link

codecov bot commented Nov 2, 2021

Codecov Report

Merging #428 (3e3f19e) into master (21bcbbc) will decrease coverage by 0.40%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #428      +/-   ##
==========================================
- Coverage   65.96%   65.55%   -0.41%     
==========================================
  Files          55       55              
  Lines        4416     4419       +3     
==========================================
- Hits         2913     2897      -16     
- Misses       1503     1522      +19     
Impacted Files Coverage Δ
lux/core/__init__.py 83.33% <66.66%> (-3.34%) ⬇️
lux/action/row_group.py 36.36% <0.00%> (-59.10%) ⬇️
lux/vislib/altair/ScatterChart.py 93.93% <0.00%> (-3.04%) ⬇️
lux/core/frame.py 75.05% <0.00%> (-0.91%) ⬇️
lux/vislib/altair/Heatmap.py 93.10% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21bcbbc...3e3f19e. Read the comment docs.

@dorisjlee dorisjlee merged commit bcb9b3d into lux-org:master Nov 3, 2021
@dorisjlee
Copy link
Member

This looks great, thanks @westernguy2!
(Note to future dev: some metadata propagation related tests are commented out here and would need more investigation for pandas> v1.3 )

@@ -1,7 +1,7 @@
scipy>=1.3.3
altair>=4.0.0
numpy>=1.16.5
pandas>=1.2.0,<1.3.0
pandas

Choose a reason for hiding this comment

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

I've also updated the requirements to require pandas>= 1.3.0

There seems to be no minimum bound here?

Copy link
Member

Choose a reason for hiding this comment

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

I agree that enforcing the pandas>=1.3.0 would be useful here, we originally took this out because we didn't want to force users into updating their pandas version from 1.2 to 1.3 because of their updates to Lux.

@westernguy2, @cgarciae : Would love to get your thoughts on this! With this PR fix, my understanding is that we no longer support pandas<1.3? Does using pandas 1.2 cause errors in Lux or fail silently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The PR fix does now support the most up to date version of pandas and pandas<1.3. In lux/core/__init__, I check for the version, so the correct variable assignment is completed based on the version.

pandas 1.2 should work as expected

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks @westernguy2 !

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