Skip to content

Conversation

erfannariman
Copy link
Member

@erfannariman erfannariman commented Jun 22, 2020

@erfannariman
Copy link
Member Author

Note that I'm trying to push a commit for the doc strings, but I can't get passed the pre-commit because of error: unused 'type: ignore' comment.

@TomAugspurger
Copy link
Contributor

but I can't get passed the pre-commit because of error: unused 'type: ignore' comment.

You can use git commit --no-verify. Sometimes mypy struggles with partial diffs.

Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

We'll need a release note in 1.1.0.rst and tests.

You should also add the same keyword to Series.explode.

@erfannariman
Copy link
Member Author

@TomAugspurger added the changes you requested for DataFrame.explod and Series.explode, will look into release note in 1.1.0.rst and tests. Not really familiar with this so will try myself first,but if I need help can I ask you some questions?

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 22, 2020 via email

@erfannariman
Copy link
Member Author

@TomAugspurger I think I added all the requests you had, will check the merge conflict, not sure how this could happen.

@jorisvandenbossche jorisvandenbossche changed the title [IMP] - #34932 - added option to ignore_index in DataFrame.explod ENH: add ignore_index option in DataFrame.explode Jun 22, 2020
@erfannariman
Copy link
Member Author

The new test fails because of differences in types:

def test_ignore_index():
        # GH 34932
        s = pd.Series([[1, 2], [3, 4]])
        result = s.explode(ignore_index=True)
        expected = pd.Series([1, 2, 3, 4], index=[0, 1, 2, 3])
>       tm.assert_series_equal(result, expected)
E       AssertionError: Attributes of Series are different
E       
E       Attribute "dtype" are different
E       [left]:  object
E       [right]: int64

@erfannariman
Copy link
Member Author

@TomAugspurger @jorisvandenbossche I think I made all the changes you guys requested, if there's anything more I can do, please let me know.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

looks good a couple of comments, ping on green.

@jreback jreback added Enhancement Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Jun 23, 2020
@jreback jreback added this to the 1.1 milestone Jun 23, 2020
@jorisvandenbossche
Copy link
Member

@erfannariman can you push the changes you made?

@erfannariman erfannariman requested a review from jreback June 24, 2020 17:18
@erfannariman
Copy link
Member Author

Not sure why this change should trigger tests/tseries/offsets/test_offsets_properties.test_on_offset_implementations to fail.

@jreback
Copy link
Contributor

jreback commented Jun 25, 2020

if you can make that change and merge master can get this in

@erfannariman
Copy link
Member Author

if you can make that change and merge master can get this in

I think I made all the changes you requested, is there anything else you would like me to do?

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

looks good, pls revert the part of the whatsnew that was changed; only your change should be in the diff. ping on green.

@erfannariman erfannariman requested a review from jreback June 26, 2020 00:05
@erfannariman
Copy link
Member Author

The check that fails seems to happen because of merging master, libreduction is now unused in core/apply.py.

@jreback jreback merged commit 9c77845 into pandas-dev:master Jun 26, 2020
@jreback
Copy link
Contributor

jreback commented Jun 26, 2020

thanks @erfannariman very nice

@erfannariman
Copy link
Member Author

thanks for reviewing and helping @jreback

fangchenli pushed a commit to fangchenli/pandas that referenced this pull request Jun 27, 2020
@erfannariman erfannariman deleted the explode_ignore_index_34932 branch July 19, 2020 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: add ignore_index argument to DataFrame.explode / Series.explode
4 participants