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

BUG: fix col iteration in DataFrame.round, #11611 #11618

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@skycaptain
Contributor

skycaptain commented Nov 16, 2015

Fixes #11611. The iterator now uses DataFrame.iteritems instead of direct indexing.

try:
yield np.round(df[col], decimals[col])
yield np.round(vals, decimals[col])

This comment has been minimized.

@max-sixty

max-sixty Nov 16, 2015

Contributor

Is the decimals lookup going to have the same problem if it's not unique here?

@max-sixty

max-sixty Nov 16, 2015

Contributor

Is the decimals lookup going to have the same problem if it's not unique here?

This comment has been minimized.

@skycaptain

skycaptain Nov 16, 2015

Contributor

The value of decimals is either of type dict or pandas.Series. Whereas the former can not have duplicate entries, a pandas.Series might have and direct indexing will return all values with the same index. So, this case is still not handled.

@skycaptain

skycaptain Nov 16, 2015

Contributor

The value of decimals is either of type dict or pandas.Series. Whereas the former can not have duplicate entries, a pandas.Series might have and direct indexing will return all values with the same index. So, this case is still not handled.

This comment has been minimized.

@jreback

jreback Nov 18, 2015

Contributor

I think let's raise if decimals is not unique (it should be a dict/Series).

@jreback

jreback Nov 18, 2015

Contributor

I think let's raise if decimals is not unique (it should be a dict/Series).

This comment has been minimized.

@skycaptain

skycaptain Nov 18, 2015

Contributor

Had the same thought this morning.

@skycaptain

skycaptain Nov 18, 2015

Contributor

Had the same thought this morning.

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 16, 2015

Contributor

need some tests

Contributor

jreback commented Nov 16, 2015

need some tests

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 18, 2015

Contributor

pls add the test example as well (you can just smoke tests it I think; e.g. assert it runs).

add a tests for passing a non-unique decimal.

put this in the bug-fix section for a whatsnew entry.

Contributor

jreback commented Nov 18, 2015

pls add the test example as well (you can just smoke tests it I think; e.g. assert it runs).

add a tests for passing a non-unique decimal.

put this in the bug-fix section for a whatsnew entry.

@skycaptain

This comment has been minimized.

Show comment
Hide comment
@skycaptain

skycaptain Nov 18, 2015

Contributor

Agreed.

Contributor

skycaptain commented Nov 18, 2015

Agreed.

@jreback jreback added this to the 0.17.1 milestone Nov 20, 2015

jreback added a commit that referenced this pull request Nov 20, 2015

BUG: fix col iteration in DataFrame.round, #11611
BUG: decimals must be unique indexed, #11618

BUG: Added test, added whatsnew entry, #11618

TST: move round testing to test_format.py
@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Nov 20, 2015

Contributor

merged via 80a2d53

thanks!

Contributor

jreback commented Nov 20, 2015

merged via 80a2d53

thanks!

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