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.

Sign up for GitHub

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

Jump to bottom

DEPR: deprecate (Sparse)Series.from_array #18258

Merged
merged 5 commits into from
Nov 17, 2017
Merged

DEPR: deprecate (Sparse)Series.from_array #18258

merged 5 commits into from
Nov 17, 2017

Conversation

jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Nov 13, 2017 •
edited
Loading

Closes #18213.

It's a bit annoying that I have to keep it as internal method. On the other hand this might signal that from_array actually has some use case ..

@jorisvandenbossche jorisvandenbossche added Deprecate Functionality to remove in pandas Sparse Sparse Data Type labels Nov 13, 2017
@jorisvandenbossche jorisvandenbossche added this to the 0.22.0 milestone Nov 13, 2017
jreback
jreback reviewed Nov 13, 2017
View reviewed changes
pandas/core/series.py Outdated
fastpath=fastpath)

@classmethod
def _from_array(cls, arr, index=None, name=None, dtype=None, copy=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

its prob easier just to elminate this and deal with SparseSeries directly in the Series constructor. Do we have an issue for this?

Copy link
Contributor

@jreback jreback Nov 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

annoyingly we'd actually have to do this in __new__ (but not that big of a deal)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that would be cleaner. But it is also an API change that Series(..) would no longer always return a Series but depending on the input also SparseSeries.
So the question is if we actually want this. I can open an issue about it though.

@jsexauer jsexauer mentioned this pull request Nov 13, 2017
Closed
1 task
@codecov
Copy link

codecov bot commented Nov 14, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@7b5ca80). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #18258   +/-   ##
=========================================
  Coverage          ?   91.38%           
=========================================
  Files             ?      164           
  Lines             ?    49892           
  Branches          ?        0           
=========================================
  Hits              ?    45594           
  Misses            ?     4298           
  Partials          ?        0
Flag Coverage Δ
#multiple 89.19% <100%> (?)
#single 39.42% <50%> (?)
Impacted Files Coverage Δ
pandas/core/frame.py 97.8% <100%> (ø)
pandas/core/series.py 95.02% <100%> (ø)
pandas/core/sparse/series.py 95.33% <100%> (ø)

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 7b5ca80...4317073. Read the comment docs.

@codecov
Copy link

codecov bot commented Nov 14, 2017 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@7b5ca80). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #18258   +/-   ##
=========================================
  Coverage          ?   91.38%           
=========================================
  Files             ?      164           
  Lines             ?    49888           
  Branches          ?        0           
=========================================
  Hits              ?    45591           
  Misses            ?     4297           
  Partials          ?        0
Flag Coverage Δ
#multiple 89.19% <100%> (?)
#single 39.42% <63.63%> (?)
Impacted Files Coverage Δ
pandas/core/frame.py 97.8% <100%> (ø)
pandas/core/series.py 95.02% <100%> (ø)
pandas/core/sparse/series.py 95.33% <100%> (ø)

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 7b5ca80...4317073. Read the comment docs.

@jreback jreback mentioned this pull request Nov 16, 2017
Closed
34 tasks
@jreback jreback merged commit 774030c into pandas-dev:master Nov 17, 2017
@jreback
Copy link
Contributor

jreback commented Nov 17, 2017

thanks!

@jorisvandenbossche jorisvandenbossche mentioned this pull request Dec 10, 2017
Closed
@gfyoung
Copy link
Member

gfyoung commented Jan 21, 2018

@jorisvandenbossche @jreback : This commit introduced the first (and only) instance of pytest.warns instead of our in-house tm.assert_produces_warning. Do we have a preference for which should be used now that we require pytest >= 3.1 ?

@jreback
Copy link
Contributor

jreback commented Jan 21, 2018

@gfyoung let's just use assert_produces_warning. and alway pls add a lint rule to fail onpytest.warns. We could of course change to use pytest.warns (maybe) but that's a different issue.

@gfyoung
Copy link
Member

gfyoung commented Jan 21, 2018

@jreback : I figured as such. Thanks for confirming.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Jan 22, 2018
@gfyoung
MAINT: Check for pytest.warns in tests
499d330 
Per discussion in pandas-devgh-18258, we are
prohibiting its use in tests, at
least for the time being.
@gfyoung gfyoung mentioned this pull request Jan 22, 2018
Merged
gfyoung added a commit to forking-repos/pandas that referenced this pull request Jan 22, 2018
@gfyoung
MAINT: Check for pytest.warns in tests
a5c0940 
Per discussion in pandas-devgh-18258, we are
prohibiting its use in tests, at
least for the time being.
jreback pushed a commit that referenced this pull request Jan 23, 2018
@gfyoung @jreback
MAINT: Remove pytest.warns in tests (#19341)
d409eaf 
* MAINT: Check for pytest.warns in tests

Per discussion in gh-18258, we are
prohibiting its use in tests, at
least for the time being.

* MAINT: Patch lint error with pytest.warns

The lint correctly fails on this line.
@jorisvandenbossche jorisvandenbossche deleted the depr-from_array branch February 23, 2018 07:50
@jorisvandenbossche jorisvandenbossche mentioned this pull request Feb 23, 2018
Open
@KalyanGokhale KalyanGokhale mentioned this pull request Jun 25, 2018
Merged
3 tasks
@jreback jreback mentioned this pull request Nov 20, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback left review comments

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas Sparse Sparse Data Type
Projects
None yet
Milestone
0.23.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jorisvandenbossche @jreback @gfyoung