PERF: avoid double copy in `to_numpy` functions if `np.asarray` is used
State: closed

As reported in #24345, there is a problem with possible double copying
in the different `to_numpy` functions implemented for some pandas
objects.

In particular, we are talking about the `to_numpy` functions that use
`np.asarray` here, because `np.asarray` might have already copied the
object if the input was not a numpy array or the requested dtype didn't
match.

In #24345, a possible solution was suggested: replacing `np.asarray`
with a custom function that reports whether a copy happened. However, we
can easily check this also when using `np.asarray` by checking object
equality (`arr_out is arr_in`), as shown in the numpy documentation [1].
This commit implements these checks to avoid the possible double copy.

In the case of the ExtensionArray implemented in
`pandas/core/arrays/base.py`, the object equality is only meaningful to
check if the ExtensionArray implements a custom array container [2], and
then we need to compare the array object returned by `np.asarray` to the
object returned by `__array__()`.

This commit also includes a small bugfix. The `arr_out is arr_in` check
was actually used already in `pandas/core/arrays/numpy_.py`, but at the
wrong place: if a copy already happened, the optional NaN value filling
was skipped. This commit fixes that.

Finally, the intendation level of some of the NaN-filling code is
changed to not redundantly check `na_value is not lib.no_default`.

[1] https://numpy.org/doc/stable/reference/generated/numpy.asarray.html
[2] https://numpy.org/devdocs/user/basics.dispatch.html

- [ ] closes #24345
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit) for how to run them
- [ ] whatsnew entry


Labels:
name: Performance
description: Memory or execution speed performance
name: Stale

Comments:
Hello @guitargeek! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:

























There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2022-02-01 23:36:47 UTC
Hi @jbrockmendel, thanks a lot for the review! I have added fixup commits where I'm addressing them with the `_to_numpy` API originally proposed in the linked issue.

Let me know what you think, if you deem it the right approach I can go ahead with also implementing `_to_numpy` for the other extension arrays.
@guitargeek can you merge master and address comments
Hi, sorry for the late reply! I'll rebase on master now and push the code.

To answer you question:

> would rather not add `_to_numpy` here, why is `__array__` not just the right place?

The problem is that there is no way to tell if `__array__` already returns a copy, and if we use it in `to_numpy` we can't avoid double-copying. If the `copy=True` argument was passed and `__array__` already returns a copy, e.g. because the dtype didn't match or because the array is of a custom pandas array class whose `__array__` always returns a copy.

That's why I proposed `_to_numpy` as part of the interface of the pandas array classes: a function equivalent to `__array__`, only that it returns an additional `Bool` that tells you if the returned array is already a copy.

If you have an idea for a alternative solution, please let me know and I'll implement it promptly!
By the way, if you would consider at one point merging this, let me know and I'll clean up the commit history before
> That's why I proposed _to_numpy as part of the interface of the pandas array classes: a function equivalent to __array__, only that it returns an additional Bool that tells you if the returned array is already a copy.

I now understand why `__array__` on its own won't work here, but I'm still not wild about this.  What about something like a `shares_memory` method (maybe on the dtype?) that could 1) be used to tell if a copy was made and 2) be re-used by tm.shares_memory?

> By the way, if you would consider at one point merging this, let me know and I'll clean up the commit history before

don't worry about it, we "Squash and merge".
@guitargeek can you merge master
This pull request is stale because it has been open for thirty days with no activity. Please [update](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#updating-your-pull-request) and respond to this comment if you're still interested in working on this.
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge the main branch, address the review comments, and we can reopen.