Skip to content

BUG: Ensure .astype doesn't use PandasArray #24866

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 28, 2019
Merged

BUG: Ensure .astype doesn't use PandasArray #24866

merged 5 commits into from
Apr 28, 2019

Conversation

TomAugspurger
Copy link
Contributor

On 0.24.0rc1, it's possible to end up with a PandasArray in internals.

In [8]: ser = pd.Series([1, 2])

In [9]: ser.astype(ser.array.dtype)._data.blocks[0]
Out[9]: ExtensionBlock: 2 dtype: int64

@TomAugspurger
BUG: Ensure .astype doesn't use PandasArray
9f74737 
On 0.24.0rc1, it's possible to end up with a PandasArray in internals.

```python
In [8]: ser = pd.Series([1, 2])

In [9]: ser.astype(ser.array.dtype)._data.blocks[0]
Out[9]: ExtensionBlock: 2 dtype: int64
```
@gfyoung gfyoung added Bug Dtype Conversions Unexpected or buggy dtype conversions ExtensionArray Extending pandas with custom dtypes or arrays. labels Jan 21, 2019
@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Jan 21, 2019 via email

@TomAugspurger TomAugspurger mentioned this pull request Jan 22, 2019
Open
@codecov
Copy link

codecov bot commented Jan 22, 2019

Codecov Report

Merging #24866 into master will decrease coverage by 49.5%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24866       +/-   ##
===========================================
- Coverage   92.39%   42.88%   -49.51%     
===========================================
  Files         166      166               
  Lines       52412    52414        +2     
===========================================
- Hits        48424    22479    -25945     
- Misses       3988    29935    +25947
Flag Coverage Δ
#multiple ?
#single 42.88% <66.66%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/internals/blocks.py 52.25% <66.66%> (-42.01%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-99.35%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a49be7f...548377f. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 22, 2019 •
edited
Loading

Codecov Report

Merging #24866 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24866      +/-   ##
==========================================
- Coverage   91.98%   91.97%   -0.02%     
==========================================
  Files         175      175              
  Lines       52372    52369       -3     
==========================================
- Hits        48172    48164       -8     
- Misses       4200     4205       +5
Flag Coverage Δ
#multiple 90.52% <100%> (-0.01%) ⬇️
#single 40.69% <50%> (-0.15%) ⬇️
Impacted Files Coverage Δ
pandas/core/internals/blocks.py 94.07% <100%> (-0.02%) ⬇️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 96.9% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.61% <0%> (-0.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48ea04f...6a37fa1. Read the comment docs.

TomAugspurger
TomAugspurger commented Jan 22, 2019
View reviewed changes
pandas/core/internals/blocks.py
# astype processing
if is_dtype_equal(self.dtype, dtype):
if copy:
return self.copy()
return self

klass = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that this is unnecessary now... The .astype should get us an array of the correct type, and from there we'll get the correct block in make_block.

@TomAugspurger
Copy link
Contributor Author

All green.

I'm a bit concerned that this is the wrong place to fix the issue. We could alternatively put this check in make_block so that if a PandasArray or PandasDtype is there, the the array / dtype is converted to a NumPy version before proceeding...

@TomAugspurger
Copy link
Contributor Author

Trying to move it to make_block.

@jreback
Copy link
Contributor

jreback commented Apr 20, 2019

needs a retest, can you merge master

@jreback jreback added this to the 0.25.0 milestone Apr 20, 2019
@jreback
Copy link
Contributor

jreback commented Apr 28, 2019

merged master; lets see

@jreback jreback merged commit 51980fe into pandas-dev:master Apr 28, 2019
@jreback
Copy link
Contributor

jreback commented Apr 28, 2019

thanks @TomAugspurger

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

@jreback jreback jreback left review comments

@gfyoung gfyoung gfyoung left review comments

@jorisvandenbossche jorisvandenbossche jorisvandenbossche approved these changes

Assignees
No one assigned
Labels
Bug Dtype Conversions Unexpected or buggy dtype conversions ExtensionArray Extending pandas with custom dtypes or arrays.
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.
4 participants
@TomAugspurger @jreback @jorisvandenbossche @gfyoung