Pypy refcheck
State: closed

``np.resize(a, refcheck=True)`` relies on refcount semantics to check that no other object holds a reference to ``a``.  Since PyPy only partially mocks refcount semantics, the check is unreliable on PyPy. Unfortunately ``a`` (or rather ``uniques`` in this case) is allocated all the way out at user space, so it might be problematic to always use ``refcheck=False``. Instead I percolated ``refcheck`` as a kwarg with default value ``True`` for safety out to the point at which ``uniques`` is allocated. See also issue #15854.

With this change, on PyPy, failing tests that use hashtables now pass


Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=h1) Report
> Merging [#16193](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/217864e3902592b9723a1070f1bbbb203a1759f4?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16193/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #16193      +/-   ##
==========================================
+ Coverage   90.86%   90.86%   +<.01%     
==========================================
  Files         162      162              
  Lines       50862    50862              
==========================================
+ Hits        46215    46216       +1     
+ Misses       4647     4646       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.64% <100%> (ø)` | :arrow_up: |
| #single | `40.3% <66.66%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/algorithms.py](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYWxnb3JpdGhtcy5weQ==) | `94.41% <100%> (ø)` | :arrow_up: |
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `93.94% <100%> (ø)` | :arrow_up: |
| [pandas/core/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tbW9uLnB5) | `91.03% <0%> (+0.34%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=footer). Last update [217864e...4acc4cd](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=h1) Report
> Merging [#16193](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/217864e3902592b9723a1070f1bbbb203a1759f4?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16193/graphs/tree.svg?token=eZ4WkYLtcO&src=pr&height=150&width=650)](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #16193   +/-   ##
=======================================
  Coverage   90.86%   90.86%           
=======================================
  Files         162      162           
  Lines       50862    50862           
=======================================
  Hits        46215    46215           
  Misses       4647     4647
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.64% <100%> (ø)` | :arrow_up: |
| #single | `40.3% <75%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/algorithms.py](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYWxnb3JpdGhtcy5weQ==) | `94.41% <100%> (ø)` | :arrow_up: |
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `93.94% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=footer). Last update [217864e...9aaf521](https://codecov.io/gh/pandas-dev/pandas/pull/16193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Fix formatting for linter
this seems like a giant change. why not just pass refcheck for object arrays only?
sorry for the commit bomb, but the ``resize`` call percolates out for all the ``Int64Vector, UInt64Vector, Float64Vector, StringVector`` classes, so either I must assume it is safe to always use refcheck=False (seems a bit dangerous) or percolate it out to where the ``{name}Vector`` object is initialized and may be held by another reference

The problem with the 'dangerous' option is that it will reallocate the memory of an object held by the user, which could cause inconsistent state in that object, but if that is what you recommend I will submit another pull request along those lines
At least in the current implementation, I think forcing `refcheck=False` would be safe?  The Vector classes allocate/own the ndarray memory, so it should always be the case that they can safely resize.
A bit more invasive, but we could also remove the ndarray object entirely from these classes until `to_array` is called.  Instead of calling `ao.resize`, realloc the underlying buffer - I think that would also have the advantage of removing the need to re-acquire the GIL on the resize calls, e.g. here.

https://github.com/pandas-dev/pandas/blob/20fda2223d5121be3f8204702b5ce1e6037e5b18/pandas/_libs/hashtable_class_helper.pxi.in#L364
> The Vector classes allocate/own the ndarray memory, so it should always be the case that they can safely resize

I added a test in commit 9aaf521 to show how refcheck semantics leak to app-level, if the user takes a reference to ``uniques`` and then calls ``get_labels`` again
Vector isn't part of the public api, but I do see your point, thanks for the example.  Maybe instead enforce that `to_array` can only called once  - roughly
```
cdef class {{name}}Vector:
    cdef bint external_view_exists = False
    # ...ommitted impl...

    cpdef to_array(self):
        if self.external_view_exists:
            raise ValueError("")
        else:
            # original impl
            self.external_view = True
```

superseded by #16224