ENH: add masked algorithm for mean() function
State: closed

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


Labels:
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.
name: Stale

Comments:
Hello @Akshatt! 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-12-28 13:40:51 UTC
See here:

https://github.com/pandas-dev/pandas/blob/4628665e53c214ae2e2efde54c4d703fb1c198ab/pandas/core/arrays/masked.py#L337-L339

You can add "mean" there
@jorisvandenbossche, 
The new mean function takes significantly less time 
![Screenshot (189)](https://user-images.githubusercontent.com/32221186/84778979-60942980-b001-11ea-982b-1d9f32d7c075.png)

Should tests be added? and if yes then where?
and same for whatsnew entry

can you add some asv's for the existing and this new case (perf metrics)
@jreback 
> pls add tests (we likely aready have some setup for sum, you can just add onto those)

Alright, so I need to add tests in 
* [test_reductions](https://github.com/pandas-dev/pandas/blob/5fdd6f50ac5ac2de939ef5b849e4c41e27e623a3/pandas/tests/reductions/test_reductions.py#L565)   
* [test_dtypes](https://github.com/pandas-dev/pandas/blob/5fdd6f50ac5ac2de939ef5b849e4c41e27e623a3/pandas/tests/arrays/integer/test_dtypes.py#L22)  
Correct me if im wrong or need to add in more tests.

> can you add some asv's for the existing and this new case (perf metrics)

Where exactly do I add the asv's in? 

This operation is already covered by

https://github.com/pandas-dev/pandas/blob/107ad154c65697be0f88d6519a9c5abe8fe29d70/asv_bench/benchmarks/series_methods.py#L250-L280

(I added Int64 in general for all ops, so including mean, when I added a masked sum/prod algorithm)

So no need to write additional benchmarks I think.

For tests, it would be good to add it here: https://github.com/pandas-dev/pandas/blob/5fdd6f50ac5ac2de939ef5b849e4c41e27e623a3/pandas/tests/reductions/test_reductions.py#L683 (we mainly need to check the behaviour on empty)

We are also going to need to decide what to return for an empty array/Series. Right now we return np.nan, but this can also be pd.NA. This touches a bit the discussion we had about NaN vs NA for the floating dtype, but I suppose for now we probably want to go with pd.NA (that's what we do now).


@jorisvandenbossche, 
> We are also going to need to decide what to return for an empty array/Series. Right now we return np.nan, but this can also be pd.NA. This touches a bit the discussion we had about NaN vs NA for the floating dtype, but I suppose for now we probably want to go with pd.NA (that's what we do now).

This would involve 
* checking if the mean is np.nan and then  
* returning pd.NA instead in the mean function itself  
yes?

> This would involve
> 
> * checking if the mean is np.nan and then
> * returning pd.NA instead in the mean function itself
>   yes?

@Akshatt You could also do a length check before any calculation as with other functions here, e.g. something like `not values.size`
> > This would involve
> > 
> > * checking if the mean is np.nan and then
> > * returning pd.NA instead in the mean function itself
> >   yes?
> 
> @Akshatt You could also do a length check before any calculation as with other functions here, e.g. something like `not values.size`

Hi @dsaxton, thanks for replying!
```
if not skipna: 
     if mask.any() or not values.size:
         return libmissing.NA
```
This would suffice, right?  
@Akshatt  Since we're always returning NA on empty input regardless of skipna could also
```python
if not values.size:
    return pd.NA

# logic for non-empty input
```
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
@Akshatt Is this still active? Can you address previous comments if so?
@Akshatt do you have time to update this? 
@jorisvandenbossche 
Looks like the tests are not passing for `float64` and `object` dtypes. They return `nan` for empty series and not `pd.NA`. Should I keep a check for them separately or remove them from the parameterized dtypes?
Yes, the "object" dtype can be removed (it's not a nullable dtype), and "float64" can be changed to "Float64", see my inline comments
do we have an asv that hits this? also need a whatsnew note for 1.3 that says perf increased for mean on an EA masked array.
> do we have an asv that hits this?

Yes, `stat_ops.py::FrameOps` and `series_methods.py::NanOps` already cover this
@Akshatt can you update
> @Akshatt can you update

@jreback, what needs to be updated?  adding `min_count` ? 
can you check if we have asv's as indicated inthe OP and show the results of them. (as this was the point of adding a specific function here)
> adding min_count ?

As I already answered, `min_count` is not applicable here. So I think it is only the whatsnew note that remains code-wise.
> > adding min_count ?
> 
> As I already answered, `min_count` is not applicable here. So I think it is only the whatsnew note that remains code-wise

this is fine (you are repeating an old commment which i agree is not relevant).

the asv's and whatsnew are still needed.
> you are repeating an old commment which i agree is not relevant

Jeff, I am answering @Akshatt because he asked exactly that 2 hours ago
Since setting up ASV is not easiest thing to do, I quickly ran the aforementioned benchmark that covers this case (`asv run --python=same -b "series_methods\.NanOps\.time_func\('mean'"` on master vs this branch):


master:

```
[100.00%] ··· series_methods.NanOps.time_func              ok
[100.00%] ··· ======== ========= ============ ============= ============= ============= ============ ============
              --                                                      dtype                                      
              ------------------ --------------------------------------------------------------------------------
                func       N         int8         int32         int64        float64       Int64       boolean   
              ======== ========= ============ ============= ============= ============= ============ ============
                mean      1000    36.3±0.3μs    21.2±0.1μs    21.0±0.1μs   21.0±0.06μs   40.7±0.2μs   46.1±0.2μs 
                mean    1000000    578±3μs     1.06±0.04ms   1.14±0.04ms    1.26±0.2ms   1.87±0.2ms   1.63±0.2ms 
              ======== ========= ============ ============= ============= ============= ============ ============

````

this branch:

```
[100.00%] ··· ======== ========= =========== ============ ============ ============ ============= =============
              --                                                     dtype                                     
              ------------------ ------------------------------------------------------------------------------
                func       N         int8       int32        int64       float64        Int64        boolean   
              ======== ========= =========== ============ ============ ============ ============= =============
                mean      1000     47.0±5μs   23.2±0.9μs    25.1±3μs    22.5±0.7μs     15.6±1μs      19.3±1μs  
                mean    1000000   779±200μs   1.23±0.3ms   1.36±0.3ms   1.46±0.4ms   1.45±0.08ms   1.38±0.03ms 
              ======== ========= =========== ============ ============ ============ ============= =============
```

So you see a modest speedup for the Int64 / boolean case (although apparently for larger data, the relative benefit diminishes)


> > you are repeating an old commment which i agree is not relevant
> 
> Jeff, I am answering @Akshatt because he asked exactly that 2 hours ago

as did i but ok
Hi, sorry to have caused all the misunderstanding.
@jorisvandenbossche, thanks for running the asv benchmark as I have no experience in doing it.
So, all that's left is to add the entry in the Whats new in `v1.3.0.rst` under Performance improvements like so 
" Performance improvement in :meth:`array.mean` (:issue:`34814`) ". is this okay? Should I attach the asv results as well?  
> So, all that's left is to add the entry in the Whats new in v1.3.0.rst under Performance improvements like so
" Performance improvement in :meth:array.mean (:issue:34814) ".

Yes, that's all.
thanks @Akshatt