BUG: Regression in astype not casting to bytes #39484
Conversation
| def test_astype_bytes(self): | ||
| # GH#39474 | ||
| result = DataFrame(["foo", "bar", "baz"]).astype(bytes) | ||
| assert result.dtypes[0] == np.dtype("S3") |
There was a problem hiding this comment.
this is not correct, it should be object
There was a problem hiding this comment.
This was np.dtype("S3") on 1.1.5 too, so wrong there too?
There was a problem hiding this comment.
Could remove the test to avoid testing wrong behavior?
There was a problem hiding this comment.
hmm yeah this is definitely wrong, we should never have a numpy string type in output.
There was a problem hiding this comment.
We have unfortunately been doing this for many releases ..
There was a problem hiding this comment.
ok @phofl let's fix this for 1.3 (the S dtypes) and backport this change.
There was a problem hiding this comment.
Ok all right. Will open issue as follow up
| @@ -2247,7 +2247,7 @@ class ObjectBlock(Block): | |||
| _can_hold_na = True | |||
|
|
|||
| def _maybe_coerce_values(self, values): | |||
| if issubclass(values.dtype.type, (str, bytes)): | |||
| if issubclass(values.dtype.type, str): | |||
There was a problem hiding this comment.
why are you changing this?
There was a problem hiding this comment.
I have added this in #39065, which caused the regression. have to handle the replace issue at another place
jreback
added
Regression
| @@ -2247,7 +2247,7 @@ class ObjectBlock(Block): | |||
| _can_hold_na = True | |||
|
|
|||
| def _maybe_coerce_values(self, values): | |||
| if issubclass(values.dtype.type, (str, bytes)): | |||
| if issubclass(values.dtype.type, str): | |||
| def test_astype_bytes(self): | ||
| # GH#39474 | ||
| result = DataFrame(["foo", "bar", "baz"]).astype(bytes) | ||
| assert result.dtypes[0] == np.dtype("S3") |
There was a problem hiding this comment.
hmm yeah this is definitely wrong, we should never have a numpy string type in output.
|
@phofl if you'd merge master |
|
merged master |
|
Opened #39566 |
|
thanks @phofl |
|
@meeseeksdev backport 1.2.x |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free to suggest an improvement. |
(cherry picked from commit 300d1fc)
Not sure if this is the right place