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

DEPR: Warn on empty Series dtype inference #20802

Closed
wants to merge 2 commits into from

Conversation

TomAugspurger
Copy link
Contributor

We're changing Series([]) to be object instead of float. This does the warning

@TomAugspurger TomAugspurger added Dtype Conversions Unexpected or buggy dtype conversions API Design labels Apr 23, 2018
@TomAugspurger TomAugspurger added this to the 0.23.0 milestone Apr 23, 2018
@TomAugspurger
Copy link
Contributor Author

FYI, d80e1c7 has the changes. Everything else is catching / fixing warning internal to pandas. This ended up being a decent-sized change.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

generally looks good. looks like you have some extra commits (so rebase on master), and a couple of style changes.

@@ -862,6 +862,7 @@ Other API Changes
Deprecations
~~~~~~~~~~~~

- The inferred dtype for an empty Series will change from ``float`` to ``object`` (:issue:`17261`).
Copy link
Contributor

Choose a reason for hiding this comment

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

double-backticks on Series

@@ -954,7 +954,11 @@ def _map_values(self, mapper, na_action=None):
# we specify the keys here to handle the
# possibility that they are tuples
from pandas import Series
mapper = Series(mapper)
if len(mapper) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be worthwhile to make a function in pandas.core.dtypes.cast.maybe_infer_1d_array_dtype

@@ -5399,7 +5399,11 @@ def fillna(self, value=None, method=None, axis=None, inplace=False,
if self.ndim == 1:
if isinstance(value, (dict, ABCSeries)):
from pandas import Series
value = Series(value)
if len(value) == 0:
series_dtype = 'float'
Copy link
Contributor

Choose a reason for hiding this comment

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

see above

name(s) to set
level : int, level name, or sequence of int/level names (default None)
If the index is a MultiIndex (hierarchical), level(s) to set (None
for all levels). Otherwise level must be None
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like you have some extra commits?

@@ -388,10 +388,14 @@ def test_map(self):

@pytest.mark.parametrize("index", tm.all_index_generator(10))
def test_map_empty(self, index):
s = Series(index)
if len(index) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

use function as above

@TomAugspurger
Copy link
Contributor Author

Sorry about the empty commits, will fix that up.

Still going through and squashing warnings. There are a few cases internally where we construct an empty series. e.g.

In [1]: import pandas as pd

In [2]: pd.Series([], dtype='object').groupby([]).apply(lambda x: 1)
Out[2]: Series([], dtype: float64)

How should we handle that?

  1. Leave as float64, specify the dtype to keep compat
  2. Leave as float64, no dtype to generate a warning

The problem w/ 2 is that there's nothing the user can do to silence the warning.

@TomAugspurger TomAugspurger modified the milestones: 0.23.0, 0.24.0 Apr 25, 2018
@jreback
Copy link
Contributor

jreback commented Sep 25, 2018

@TomAugspurger can you close or update

@TomAugspurger
Copy link
Contributor Author

@jreback @jorisvandenbossche do you think we want to change this before 1.0?

@jreback
Copy link
Contributor

jreback commented Nov 18, 2018

@TomAugspurger can you merge master

@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Nov 19, 2018 via email

@jreback jreback removed this from the 0.24.0 milestone Nov 20, 2018
@jreback
Copy link
Contributor

jreback commented Jan 14, 2019

@TomAugspurger keep this one open?

@TomAugspurger
Copy link
Contributor Author

Probably not :/ I suppose we can just live with this. A little artifact of the time Series used to be an ndarray.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Dtype Conversions Unexpected or buggy dtype conversions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants