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

Infer Decimal dtype #15690

Closed
dhirschfeld opened this Issue Mar 15, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@dhirschfeld
Contributor

dhirschfeld commented Mar 15, 2017

In [15]: from decimal import Decimal

In [16]: s = pd.Series([Decimal(3), Decimal(1), Decimal(4)])
    ...: s
Out[16]: 
0    3
1    1
2    4
dtype: object

In [17]: s.dtype
Out[17]: dtype('O')

In [18]: pd.lib.infer_dtype(s)
Out[18]: 'mixed'

In [19]: set(map(type, s))
Out[19]: {decimal.Decimal}

It would be nice if the infer_dtype function could determine if the column was all of decimal type which could be useful when working with databases which return data as decimal types

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Mar 15, 2017

Contributor

you can do this in master

In [2]: from pandas.types.common import is_decimal

n [4]: s.apply(is_decimal).all()
Out[4]: True

your suggestion is quite easy to do if you'd like to submit a PR! (FYI, pandas._libs/src/inference.pyx is the new home of infer_dtype)

Contributor

jreback commented Mar 15, 2017

you can do this in master

In [2]: from pandas.types.common import is_decimal

n [4]: s.apply(is_decimal).all()
Out[4]: True

your suggestion is quite easy to do if you'd like to submit a PR! (FYI, pandas._libs/src/inference.pyx is the new home of infer_dtype)

@jreback jreback added this to the Next Major Release milestone Mar 15, 2017

@margaret

This comment has been minimized.

Show comment
Hide comment
@margaret

margaret May 22, 2017

Contributor

I'm going to work on this (pycon sprints)

Contributor

margaret commented May 22, 2017

I'm going to work on this (pycon sprints)

margaret added a commit to margaret/pandas that referenced this issue May 22, 2017

@jorisvandenbossche

This comment has been minimized.

Show comment
Hide comment
@jorisvandenbossche

jorisvandenbossche May 22, 2017

Member

@jreback Repeating my comment from on the PR: What I am wondering is what are potential consequences of such a change. As there are quite some places in the codebase where we wheck if the inferred type is 'mixed', so changing this would mean potential changes in the taken code path for Decimal objects (with potential changes in behaviour).
But don't know if those cases in the code would be relevant for Decimal objects.

Member

jorisvandenbossche commented May 22, 2017

@jreback Repeating my comment from on the PR: What I am wondering is what are potential consequences of such a change. As there are quite some places in the codebase where we wheck if the inferred type is 'mixed', so changing this would mean potential changes in the taken code path for Decimal objects (with potential changes in behaviour).
But don't know if those cases in the code would be relevant for Decimal objects.

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback May 22, 2017

Contributor

@jorisvandenbossche #16426 (comment)

after the PR is merged, should create an issue to use the new inference (though as I pointed, maybe not that useful ATM).

Contributor

jreback commented May 22, 2017

@jorisvandenbossche #16426 (comment)

after the PR is merged, should create an issue to use the new inference (though as I pointed, maybe not that useful ATM).

@jreback jreback modified the milestones: 0.21.0, Next Major Release May 23, 2017

jreback added a commit that referenced this issue May 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment