DEPR: .take(..., is_copy=True); rename is_copy -> copy

[jreback]

this is more in-line with our other signatures. ideally would do this for 0.25

[jorisvandenbossche]

Or simply deprecate this without replacement? A take operation is in principle always a copy

[jreback]

you can try to deprecate it entirely if that works

[jorisvandenbossche]

So the special case for "no copy" is when the result is exactly the same as the caller. Personally, I don't think we need to provide this special case (if the user cares about the performance in that corner case, they can do the check themselves)

[chrisstpierre]

I opened a PR to deprecate it in 0.25.2. is_copy is supposed to be removed all-together in 1.0.0.

take always returns a copy

pandas/pandas/core/generic.py

Lines 3437 to 3439 in c4489cb

	new_data = self._data.take(
	indices, axis=self._get_block_manager_axis(axis), verify=True
	)

pandas/pandas/core/internals/managers.py

Lines 1394 to 1396 in c4489cb

	return self.reindex_indexer(
	new_axis=new_labels, indexer=indexer, axis=axis, allow_dups=True
	)

pandas/pandas/core/internals/managers.py

Lines 1222 to 1224 in c4489cb

	def reindex_indexer(
	self, new_axis, indexer, axis, fill_value=None, allow_dups=False, copy=True
	):

Copy is true by default.

[jorisvandenbossche]

@chrisstpierre can you open a new PR against master? (using the same branch, but not selecting the 0.25.x branch when opening the PR)
I looked at the commit you added, and that by itself looks good!

[chrisstpierre]

@jorisvandenbossche just did that.

But I still have .. deprecated:: 0.25.2.

Should it be 1.0.0 ?

[TomAugspurger]

There's a stalled PR at #28751 that provides a nice starting point.

[ryankarlos]

take

[jorisvandenbossche]

Reopening this issue (to have an issue with a milestone) to track the discussion in the merged PR: #30615 (comment)

I think that the current deprecation is keeping the "wrong" behaviour, as in my head, the result of take is always a copy, so we shouldn't set _is_copy IMO.
(but I also might be wrong, it's pretty confusing, and the is_copy keyword has also a very confusing name and unclear explanation of what it is actually intended to do)

[TomAugspurger]

@jorisvandenbossche do you think this is a blocker for the RC?

[jorisvandenbossche]

Ah, forgot this one, we should have discussed it yesterday on the call ..

Well, I personally think the merged PR is a step in the wrong direction. It's easy to revert that, but IMO the better solution is to make take do a proper copy. But that is 1) something that needs to be discussed and 2) is more work to implement.

[jorisvandenbossche]

Given the lack of input of others, shall we revert the PR for the RC?

[jreback]

not sure what exactly is the issue you are bringing up here; we deprecated is_copy and you reopened exactly why?

[jorisvandenbossche]

not sure what exactly is the issue you are bringing up here; we deprecated is_copy and you reopened exactly why?

Right now, after the deprecation, we will keep is_copy=True, while I think it should be is_copy=False that becomes the default. Please read #27357 (comment) and #30615 (comment) (and comments below)

[jorisvandenbossche]

Example consequence of the PR. Using sample() can now give a false-positive warning:

In [11]: df = pd.DataFrame(np.random.randn(10,3), columns=['a', 'b', 'c'])   

In [12]: df2 = df.sample(3)  

In [14]: df2['d']= 1   
/home/joris/miniconda3/envs/dev/bin/ipython:1: SettingWithCopyWarning: 
A value is trying to be set on a copy of a slice from a DataFrame.
Try using .loc[row_indexer,col_indexer] = value instead

See the caveats in the documentation: https://pandas.pydata.org/pandas-docs/stable/user_guide/indexing.html#returning-a-view-versus-a-copy
  #!/home/joris/miniconda3/envs/dev/bin/python

The above is on master, on 0.25.3 this did not raise a warning.

[jorisvandenbossche]

I updated my PR that made take to always copy to include an internal take with the old behaviour to be used in the indexing code: #30784