Skip to content

Fix fillna accepting downcast as dict #40861

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 13, 2021
Merged

Conversation

phofl
Copy link
Member

@phofl phofl commented Apr 9, 2021

This fix makes sense in this case I think, but this does not fix the case for non-list like values, which raises a NotImplementedError

The docstring for Series.fillna says, that downcast may be a dict too, but I don't think that this makes sense, so we should maybe raise an error there?

@jbrockmendel
Copy link
Member

This fix makes sense in this case I think, but this does not fix the case for non-list like values, which raises a NotImplementedError

by coincidence i've been looking at Block.downcast and found that in the 1D case all test cases have dtypes in [None, "infer"] (and the None case defaults to "infer"), and as you noted the 2D case only accepts "infer". Anything we can do to be stricter and/or get downcast out of Block would be a plus by my lights

@phofl
Copy link
Member Author

phofl commented Apr 13, 2021

Could deprecate the downcast keyword?

@jbrockmendel
Copy link
Member

Could deprecate the downcast keyword?

id be on board for this, no idea about others.

@jreback jreback added Bug Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate labels Apr 13, 2021
@jreback jreback added this to the 1.3 milestone Apr 13, 2021
@jreback jreback merged commit b4e9566 into pandas-dev:master Apr 13, 2021
@jreback
Copy link
Contributor

jreback commented Apr 13, 2021

thanks @phofl

@jreback
Copy link
Contributor

jreback commented Apr 13, 2021

Could deprecate the downcast keyword?

id be on board for this, no idea about others.

yeah +1 here as well. this was an artifact from a while back when this was expensive. we should always just downcast (as that is what we do everywhere else)

@phofl
Copy link
Member Author

phofl commented Apr 14, 2021

Open an issue about this or directly a pr?

@phofl phofl deleted the 40809 branch April 14, 2021 20:15
@jbrockmendel
Copy link
Member

Open an issue about this or directly a pr?

probably merits an issue, since im not sure @jreback and i are on the same page. my immediate interest is in getting the downcast keyword out of Block.fillna (and ideally Block.interpolate), which i think is distinct from what he has in mind

@phofl
Copy link
Member Author

phofl commented Apr 16, 2021

opened #40988

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: AttributeError when using dict downcast in DataFrame.fillna
3 participants