BUG: revert assert_numpy_array_equal to before c2ea8fb2 (accept non-ndarrays)
State: closed

- [x] tests added / passed
- [x] passes `git diff upstream/master | flake8 --diff`

`assert_numpy_array_equal`'s docstring is at odds with (and the existence of the `obj` argument clashes with) the fact that it's now strict on its arguments being ndrarrays.

There are [practical reasons](https://github.com/pydata/pandas/pull/13205#discussion_r65358471) to allow other kinds of arguments.

On the other hand, I don't see any reason for being strict, but I assume @sinhrks 's [PR](https://github.com/pydata/pandas/pull/13311) had some rationale behind: so a possible solution is an argument `ensure_ndarray=True` which restores the previous behaviour when set to `False`.


Labels:
name: Testing
description: pandas testing functions or related to the test suite

Comments:
## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/13355?src=pr) is **84.23%**

> Merging [#13355](https://codecov.io/gh/pydata/pandas/pull/13355?src=pr) into [master](https://codecov.io/gh/pydata/pandas/branch/master?src=pr) will increase coverage by **<.01%**

``` diff
@@             master     #13355   diff @@
==========================================
  Files           138        138          
  Lines         50721      50725     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          42723      42727     +4   
  Misses         7998       7998          
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last updated by [0c6226c...53ad5b1](https://codecov.io/gh/pydata/pandas/compare/0c6226cbbc319ec22cf4c957bdcc055eaa7aea99...53ad5b1cb30ed8a5284549e98b4987d2f42f2973)

-1 to allow non-ndarray only for a single issue. How about add check_dtype arg?

you can't simply change the test procedure. The entire point is to make tests _more_ strick. So pls change as to @sinhrks suggestion.

Ideally we would be extremely strict on everything. The better checking that we have on tests the less ambiguity and the less that slips thru. The original PR did a lot to correct this.

We want strict _tests_, this is a _function_. Which I'm not "changing", except for noticing that recent changes made its behaviour incoherent with docstring and arguments.

So leaving aside general lectures on fundamentals of computation, @sinhrks what's your suggestion? There already is a `check_dtype` arg... are you saying `check_dtype=False` should mean "don't even verify whether it's a numpy array"?

(fine for me, but we loose the possibility of comparing numpy arrays of different dtypes by still ensuring they are numpy arrays)

Sorry, forget about `check_dtype`. I may not understand original discussion properly, can you show a test case which should be succeed in this case?

So, 4 options:

1) the current behaviour:

```
assert_numpy_array_equal(array([1,2]), [1, 2], check_dtype=WHATEVER) #Fails
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_dtype=False) #Passes
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_dtype=True) #Fails
```

2) the previous behaviour/current docstring/current PR:

```
assert_numpy_array_equal(array([1,2]), [1, 2], check_dtype=WHATEVER) #Passes
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_dtype=False) #Passes
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_dtype=True) #Fails
```

3) my proposal (new argument `check_ndarray`, presumably defaulting to `True`):

```
assert_numpy_array_equal(array([1,2]), [1, 2], check_ndarray=False, check_dtype=WHATEVER) #Passes
assert_numpy_array_equal(array([1,2]), [1, 2], check_ndarray=True, check_dtype=WHATEVER) #Fails
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_ndarray=WHATEVER, check_dtype=False) #Passes
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_ndarray=WHATEVER, check_dtype=True) #Fails
```

4) my interpretation of @sinhrks 's suggestion (making the existing `check_dtype`, which defaults to `True`, more strict):

```
assert_numpy_array_equal(array([1,2]), [1, 2], check_dtype=False) #Passes
assert_numpy_array_equal(array([1,2]), [1, 2], check_dtype=True) #Fails
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_dtype=False) #Passes
assert_numpy_array_equal(array([1,2]), array([1.0, 2]), check_dtype=True) #Fails
```

If we stick with 1), it probably makes more sense to write a new function `assert_is_copy`, which would work on non-`ndarray`s too (as I was previously doing in my other PR), than to add the `check_same` argument to `assert_numpy_array_equal` as I am doing now.

I personally don't have strong preferences, we should just be coherent.

Closed by #13025, thx:)

@sinhrks : this is maybe closed by you implicitly stating "we like the current behaviour" (option 1), but certainly not by https://github.com/pydata/pandas/issues/13205 (see [this comment](https://github.com/pydata/pandas/pull/13205#issuecomment-223796269) ) ;-) Which is fine by the way, if this is what you intended.

Ah I misunderstood #13025 covers curent needs.

Personally I'm +1 for option 1, but reopen for feedback from others.

yeah I think option 1 is fine, which is current behavior I think. @toobaz the cases you had worked around in #13205 look ok to me. If you find additional issues or think API needs changing, pls make a new issue (or we can discuss here).