BUG: value_counts not working correctly on ExtensionArrays
State: closed

- [ ] closes #33172
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
This could be an opportunity to use the more performant sum function from core\array_algos\masked_reductions.py and add a sum method to IntegerArray (or maybe BaseMaskedArray) There is a precedent for adding sum methods to extension arrays (timedeltas and sparse) but not sure how this fits in with the EA interface. @jorisvandenbossche?
Hello @kotamatsuoka! 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 2020-04-30 23:45:49 UTC
> This could be an opportunity to use the more performant sum function from core\array_algos\masked_reductions.py and add a sum method to IntegerArray (or maybe BaseMaskedArray) There is a precedent for adding sum methods to extension arrays (timedeltas and sparse) but not sure how this fits in with the EA interface. @jorisvandenbossche?

@simonjayhawkins That's the approach I was taking here although am not sure at the moment how best to implement this: https://github.com/pandas-dev/pandas/pull/33538
hmm, didn't realise that this PR overlaps with #33538.
@simonjayhawkins 
> the bug is fixed on master, see #33172 (comment)
> just needs tests.

I've done this.
There are 20 files with changes. only need to add a test.
@simonjayhawkins 

> pandas/tests/extension/base/methods.py: def test_value_counts(self, all_data, dropna)
> instead which generically tests all EAs for value_counts

I' added test in test_value_counts, but existing part int test_value_counts doesn't work:

```pandas/tests/extension/base/methods.py
def test_value_counts(self, all_data, dropna):
    all_data = all_data[:10].unique()
    if dropna:
        other = np.array(all_data[~all_data.isna()])
    else:
        other = all_data

    result = pd.Series(all_data).value_counts(dropna=dropna).sort_index()
    expected = pd.Series(other).value_counts(dropna=dropna).sort_index()

    self.assert_series_equal(result, expected)
```

```
[result]
left = H    1
M    1
V    1
e    1
j    1
n    1
u    1
v    1
x    1
z    1
dtype: Int64

[expected]
right = H    1
M    1
V    1
e    1
j    1
n    1
u    1
v    1
x    1
z    1
dtype: int64, args = (), kwargs = {}
```

dtypes are different, so I fixed related methods and tests.
> I' added test in test_value_counts, but existing part int test_value_counts doesn't work:

This is because the PR is also changing functionality. The idea would be to use the functionality that's already on master (for which the existing tests should be passing) and add additional test code on top of that.
@dsaxton 
> The idea would be to use the functionality that's already on master (for which the existing tests should be passing)

test_string is skipping test_value_counts. 

pandas/tests/extension/test_string.py
```pandas/tests/extension/test_string.py
class TestMethods(base.BaseMethodsTests):
    @pytest.mark.skip(reason="returns nullable")
    def test_value_counts(self, all_data, dropna):
        return super().test_value_counts(all_data, dropna)
```

Is is OK that test_string is keeping skip and add other test about normalize?
@jreback 
I removed unnecessary changes about changing dtypes in many places and forcing to Int64.
this also needs a whatsnew note, add in bug fixes, extension array section in 1.1
@kotamatsuoka the base test that you added is now being skipped for several ExtensionArray classes (which is fine, to be clear), but I think that we should then test it specifically for at least some of those classes with a custom test (I would at least test it for IntegerArray and BooleanArray). 

So instead of xfailing or skipping the test in `test_boolean.py` and `test_integer.py`, I would actually have a custom version of the test there (that has a different `expected` value, to match the behaviour).

(or is this already tested elsewhere?)
Thanks @jorisvandenbossche .
I updated test_value_counts_with_normalize(), so xfailing or skipping the test in `test_boolean.py` , `test_integer.py` and `test_sparse.py` are no longer needed。
thanks @kotamatsuoka