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

ENH: Add inplace option to drop and dropna #5247

Merged
merged 1 commit into from Oct 28, 2013

Conversation

jtratner
Copy link
Contributor

And appropriately invalidates caches as necessary BUT still makes a copy. @jreback and I decided that it shouldn't call __finalize__ a second time in _update_inplace (so if metadata is supposed to change, it's not going to be updated, but subclass can always override it). Closes #1960, related #2325 (doesn't actually fix because still makes a copy).

Adds _update_inplace method that takes a result and invalidates the internal cache then reindexes.


@jreback - would you mind taking a look? It's almost there, I think I'm
missing one cache reference or something... (or I'm just using reindex
wrong).

@jreback
Copy link
Contributor

jreback commented Oct 17, 2013

I'll have a look tomorrow

@jtratner
Copy link
Contributor Author

ping @jreback.

@jseabold - I think this covers what you were thinking. Figured it was better to just stick with the inplace idiom.

@jtratner
Copy link
Contributor Author

Updated the note to reflect that this still copies, it just updates the wrapper rather than changing the internals in place, so doesn't avoid "2X" problem referenced in #2325.

agg_axis_name = self._get_axis_name(agg_axis)
agg_obj = self.reindex(**{agg_axis_name: subset})
else:
axis = self._get_axis_number(axis)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know Panel overrides this method, but this is not safe...use get_block_manager_axis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is on DataFrame not generic, before I change, to be clear you mean changeagg_axis = 1 - axis to:

agg_axis = self._get_block_manager_axis(axis)

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry I thought it was on generic ; there is an agg_axis method somewhere (look at _reduce) - it gives same answer obviously but is more correct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because of what it needs to do, I think it's actually clearer to leave as is (because it needs to pass the name to reindex, etc.), otherwise have to either add a _get_agg_axis_number method, or add another keyword argument to _get_agg_axis.

I'm fine with going with what you'd like to do though.

Copy link
Contributor

Choose a reason for hiding this comment

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

up2u

Copy link
Contributor Author

Choose a reason for hiding this comment

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

going to keep as-is for now rather than create a separate method.

@jtratner
Copy link
Contributor Author

@jreback you okay with this now?

@jreback
Copy link
Contributor

jreback commented Oct 28, 2013

ok

jtratner added a commit that referenced this pull request Oct 28, 2013
ENH: Add inplace option to drop and dropna
@jtratner jtratner merged commit 71e2f5d into pandas-dev:master Oct 28, 2013
@jtratner jtratner deleted the inplace-drop branch October 28, 2013 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inplace dropna?
2 participants