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

BUG: setting dt64 values into Series[int] incorrectly casting dt64->int #39266

Merged
merged 8 commits into from
Jan 28, 2021

Conversation

jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel commented Jan 19, 2021

  • closes #xxxx
  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them
  • whatsnew entry

@jbrockmendel jbrockmendel added the Indexing Related to indexing on series/frames, not to indexes themselves label Jan 19, 2021
if is_list_like(value):
value = list(value)
else:
value = [value] * len(values[indexer])
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe use np.full?

Copy link
Member Author

Choose a reason for hiding this comment

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

same as above re numpy#12550

# https://github.com/numpy/numpy/issues/12550
# numpy will incorrect cast to int if we're not careful
if is_list_like(value):
value = list(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

np.asarray?

Copy link
Member Author

Choose a reason for hiding this comment

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

until numpy#12550 is fixed this is the only way i found to get this to work

@@ -991,6 +991,18 @@ def setitem(self, indexer, value):

# set
else:
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

elif?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, we still want to fall through to L1006

Copy link
Contributor

Choose a reason for hiding this comment

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

sure but i thin it is much more clear to use an elif and then simply repeat that line in the else

Copy link
Member Author

Choose a reason for hiding this comment

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

updated + green

@@ -991,6 +991,18 @@ def setitem(self, indexer, value):

# set
else:
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

sure but i thin it is much more clear to use an elif and then simply repeat that line in the else

@jbrockmendel
Copy link
Member Author

sure but i thin it is much more clear to use an elif and then simply repeat that line in the else

ok, will update.

BTW #38709, #39044, #39163 also touch this code, will need to get those merged/rejected in order to move forward on the refactor in #39302.

(and a very gentle ping on #38992 since I want to write more tests)

@jreback jreback added this to the 1.3 milestone Jan 22, 2021
@jbrockmendel
Copy link
Member Author

rebased + green

@jreback
Copy link
Contributor

jreback commented Jan 28, 2021

lgtm. can you merge master and ping on green

@jreback jreback merged commit b66160d into pandas-dev:master Jan 28, 2021
@jbrockmendel jbrockmendel deleted the bug-setitem-dt64-int branch January 28, 2021 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants