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

Backport PR #47327 on branch 1.4.x (REGR: Fix fillna making a copy when dict was given as fill value and inplace is set) #47448

Merged

Conversation

simonjayhawkins
Copy link
Member

Backport PR #47327

…t was given as fill value and inplace is set
@simonjayhawkins
Copy link
Member Author

@phofl XPASS on test_fillna_dictlike_value_duplicate_colnames. will fix-up later or feel free to push to this branch.

@phofl
Copy link
Member

phofl commented Jun 21, 2022

This is interesting, do you know if this was an intended change? Otherwise we should open an issue about it

@simonjayhawkins
Copy link
Member Author

This is interesting, do you know if this was an intended change? Otherwise we should open an issue about it

not sure. As this is 1.4.x, I'm not sure array manager changes are a concern. @jbrockmendel ?

@phofl
Copy link
Member

phofl commented Jun 22, 2022

Sorry that wasn't clear. I meant: If this is passing on 1.4.x and failing on main, this looks like a regression on main compared to the 1.4 release

@simonjayhawkins
Copy link
Member Author

yeah. can open an issue for main. for this branch, i'm sure we just skip any failing array manager tests to get ci green.

@simonjayhawkins
Copy link
Member Author

simonjayhawkins commented Jun 22, 2022

if there is a problem and a fix on main, we could then maybe consider backporting the fix for 1.4.4

@@ -146,3 +148,14 @@ def test_update_with_different_dtype(self):

expected = DataFrame({"a": [1, 3], "b": [np.nan, 2], "c": ["foo", np.nan]})
tm.assert_frame_equal(df, expected)

@td.skip_array_manager_invalid_test
Copy link
Member

Choose a reason for hiding this comment

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

should this be skip_array_manager_not_implemented? same above?

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe for the two added tests. but this is the backport. so should change on main for the change to be relevant.

for test_fillna_dictlike_value_duplicate_colnames it is XPASSing so we need to skip as skip_array_manager_not_implemented xfails.

@jbrockmendel
Copy link
Member

not sure. As this is 1.4.x, I'm not sure array manager changes are a concern. @jbrockmendel ?

i wont lose any sleep over array manager tests failing

@simonjayhawkins simonjayhawkins merged commit 8f846ae into pandas-dev:1.4.x Jun 22, 2022
@simonjayhawkins simonjayhawkins deleted the backport-of-pr-47327-on-1.4.x branch June 22, 2022 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants