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

EA: fillna should accept same type #32414

Closed
jbrockmendel opened this issue Mar 3, 2020 · 11 comments · Fixed by #46161
Closed

EA: fillna should accept same type #32414

jbrockmendel opened this issue Mar 3, 2020 · 11 comments · Fixed by #46161
Labels
Categorical Categorical Data Type ExtensionArray Extending pandas with custom dtypes or arrays. good first issue Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Needs Tests Unit test(s) needed to prevent regressions

Comments

@jbrockmendel
Copy link
Member

cat = pd.Categorical(["A", "B", None, "A"])
ser = pd.Series(cat).fillna("B")

>>> filled = cat.fillna(ser)  # <-- works

>>> cat.fillna(filled)
TypeError: 'value' parameter must be a scalar, dict or Series, but you passed a Categorical
@ghost
Copy link

ghost commented Mar 3, 2020

@jbrockmendel Is this issue appropriate for first-time contributors?

@jbrockmendel
Copy link
Member Author

Is this issue appropriate for first-time contributors?

Probably not; it gets pretty deep into the weeds. #32402 might be a good fit if you're comfortable with cython, otherwise i suggest searching for the Good First Issue label

@jbrockmendel jbrockmendel added the ExtensionArray Extending pandas with custom dtypes or arrays. label Mar 5, 2020
@jbrockmendel jbrockmendel added the Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate label Sep 21, 2020
@mroeschke
Copy link
Member

I think this work on master. Could use a test

In [22]: cat = pd.Categorical(["A", "B", None, "A"])
    ...: ser = pd.Series(cat).fillna("B")

In [23]: >>> filled = cat.fillna(ser)

In [24]: >>> cat.fillna(filled)
Out[24]:
['A', 'B', 'B', 'A']
Categories (2, object): ['A', 'B']

@mroeschke mroeschke added good first issue Needs Tests Unit test(s) needed to prevent regressions labels Jul 29, 2021
@Bhavay-2001
Copy link

hey @mroeschke , Could you please tell on which cases should we really test it on?

@mroeschke
Copy link
Member

@Bhavay192 This entire code snippet should be tested

@Bhavay-2001
Copy link

Hey @mroeschke , thanks for replying. Soo according to you I should take different values of numpy arrays and test it on that??

@mroeschke
Copy link
Member

The exact code snippet above should be written as a unit test and added to the testing suite: https://pandas.pydata.org/pandas-docs/stable/development/contributing_codebase.html#writing-tests

@Bhavay-2001
Copy link

Hey @mroeschke, Since I'm quite new to open source and unit testing. Could you please help with in finding out what all test should I Perform on it??
I can take test of input values and what should be the expected output. Any other test that I can perform??

@Bhavay-2001
Copy link

Hey @mroeschke, I have written a small unit test for the above script. Now how will you review it please tell. Shall I send a PR?? Also please tell in which package shall I add my test into??

@mroeschke
Copy link
Member

You can follow this guide to submit a PR and maintainers can help provide further guidance: https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#contributing-your-changes-to-pandas

@jreback jreback added this to the 1.4 milestone Aug 21, 2021
@jreback jreback added the Categorical Categorical Data Type label Aug 21, 2021
@simonjayhawkins
Copy link
Member

removing milestone

@simonjayhawkins simonjayhawkins removed this from the 1.4 milestone Jan 20, 2022
@weikhor weikhor mentioned this issue Feb 26, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Categorical Categorical Data Type ExtensionArray Extending pandas with custom dtypes or arrays. good first issue Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Needs Tests Unit test(s) needed to prevent regressions
Projects
None yet
5 participants