Skip to content

Removing support for deprecated pandas SparseDataFrame - #897

Merged
mfeurer merged 12 commits into
developfrom
fix_836
Mar 6, 2020
Merged

Removing support for deprecated pandas SparseDataFrame#897
mfeurer merged 12 commits into
developfrom
fix_836

Conversation

@Neeratyoy

Copy link
Copy Markdown
Contributor

Reference Issue

Addresses #836.

What does this PR implement/fix? Explain your changes.

All support for pandas SparseDataFrame have been removed. OpenML users will have to upgrade to Pandas 1.0.0 or higher to handle sparse dataframes, or as the pandas migration implies, data frames with sparse entries.

How should this PR be tested?

For pandas version >= 1.0.0
sparse_dataset = openml.datasets.get_dataset(4136, download_data=False)
rval, *_ = sparse_dataset.get_data()
print(rval.info())

For pandas version < 1.0.0
sparse_dataset = openml.datasets.get_dataset(4136, download_data=False)
rval, *_ = sparse_dataset.get_data()
Exception: Current pandas version found 0.25.3. OpenML supports pandas 1.0.0 or higher.

@codecov-io

codecov-io commented Feb 11, 2020

Copy link
Copy Markdown

Codecov Report

Merging #897 into develop will decrease coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## develop #897 +/- ##
===========================================
- Coverage 88.13% 88.06% -0.07% 
===========================================
Files 37 37 Lines 4364 4364 ===========================================
- Hits 3846 3843 -3 - Misses 518 521 +3
Impacted FilesCoverage Δ
openml/datasets/dataset.py85.98% <100%> (-0.04%)⬇️
openml/datasets/functions.py93.84% <100%> (ø)⬆️
openml/_api_calls.py87.93% <0%> (-2.59%)⬇️
openml/__init__.py100% <0%> (ø)⬆️

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 07d429c...dc8d8aa. Read the comment docs.

@mfeurer

Copy link
Copy Markdown
Collaborator

We'd need to decide whether we drop Python3.5 support for this one. Pandas 1.X.X only supports Python3.6 or higher, but has some major changes. I don't think we can and should invest the time of supporting both old and new versions of pandas, and should therefore drop Python3.5 support. @janvanrijn@amueller@PGijsbers

@PGijsbers

PGijsbers commented Feb 13, 2020

Copy link
Copy Markdown
Collaborator

In favor (and let's use f-strings!).

Comment threaddoc/progress.rst Outdated
Comment threadopenml/datasets/dataset.py Outdated
Comment threadsetup.py
Comment threadtests/test_datasets/test_dataset.py Outdated
Comment threadtests/test_datasets/test_dataset.py Outdated
Comment threadtests/test_datasets/test_dataset.py Outdated
Comment threadtests/test_datasets/test_dataset_functions.py Outdated
@Neeratyoy
Neeratyoy requested a review from mfeurerMarch 2, 2020 15:47

@mfeurermfeurer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks good, but could you please have a look at the failing unit test?

Comment threadopenml/datasets/functions.py Outdated
if hasattr(data, "columns"):
if isinstance(data, pd.SparseDataFrame):
data = data.to_coo()
# if isinstance(data, pd.SparseDataFrame):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you please remove this commented line?

@Neeratyoy

Copy link
Copy Markdown
ContributorAuthor

@mfeurer how do I re-trigger an appveyor test exclusively?

@mfeurer

Copy link
Copy Markdown
Collaborator

There used to be a button. I must admit I can only see one year old builds. @PGijsbers do you see any recent builds?

@PGijsbers

Copy link
Copy Markdown
Collaborator

image

This is what I saw. I pressed rebuild commit.

@Neeratyoy

Copy link
Copy Markdown
ContributorAuthor

@PGijsbers I think I wasn't able to see that option despite signing in. Is there something else that needs to be done to enable that?
also, thanks!

@mfeurer all test cases pass now.

@Neeratyoy
Neeratyoy requested a review from mfeurerMarch 4, 2020 16:18

@mfeurermfeurer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks great, but there are two places where I think you should update the examples a bit more. Could you please have a look at them?

@PGijsbers that button also wasn't there for me, although it used to be there...weird...

# Dataset is a pandas sparse dataframe
# ====================================

# sparse_data = coo_matrix((

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please do not keep old code.

@@ -286,12 +286,16 @@
# Dataset is a pandas sparse dataframe

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could you please update this header to reflect the fact that pandas doesn't have this class any more.

@PGijsbers

Copy link
Copy Markdown
Collaborator

From the Appveyor interface it looks like @mfeurer is an administrator and organization access it granted to Appveyor. It does say "AppVeyor is authorized to act on behalf of PGijsbers GitHub account with admin:repo_hook, read:org, repo:status scope. ".

Let's see if we can figure out what's wrong at the hackathon. I think it would be easier when we can immediately see both sides.

@Neeratyoy
Neeratyoy requested a review from mfeurerMarch 6, 2020 12:40
@mfeurer
mfeurer merged commit 249abc9 into developMar 6, 2020
@mfeurer
mfeurer deleted the fix_836 branch March 6, 2020 13:01
@mfeurermfeurer mentioned this pull request Mar 6, 2020
Sign up for freeto 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.

4 participants

@Neeratyoy@codecov-io@mfeurer@PGijsbers