ENH: add ignore index to DataFrame / Series.sample
State: closed

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


Labels:
name: Enhancement
name: Stale

Comments:
Hello @erfannariman! 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-31 17:42:29 UTC
Not sure why this fails in CI:

```
E   AttributeError: module 'numpy.random' has no attribute 'Generator'
```

Locally this seems to work:

```python
In [1]: import numpy as np

In [2]: from typing import Union

In [3]: np.random.Generator
Out[3]: numpy.random._generator.Generator

In [4]: RandomState = Union[np.random.Generator, np.random.RandomState]
```
> Not sure why this fails in CI:
> 
> ```
> E   AttributeError: module 'numpy.random' has no attribute 'Generator'
> ```
> 
> Locally this seems to work:
> 
> ```python
> In [1]: import numpy as np
> 
> In [2]: from typing import Union
> 
> In [3]: np.random.Generator
> Out[3]: numpy.random._generator.Generator
> 
> In [4]: RandomState = Union[np.random.Generator, np.random.RandomState]
> ```

This looks relevant: https://github.com/aleju/imgaug/issues/537

> This looks relevant: [aleju/imgaug#537](https://github.com/aleju/imgaug/issues/537)

Might be, but then again that topic looks quite old, and I am using the latest numpy version (1.19.4)
> 
> 
> > This looks relevant: [aleju/imgaug#537](https://github.com/aleju/imgaug/issues/537)
> 
> 
> 
> Might be, but then again that topic looks quite old, and I am using the latest numpy version (1.19.4)

Right and it works for you but what's the numpy version on the failing CI task
> Right and it works for you but what's the numpy version on the failing CI task

Yup, you are probably right, we are using version 1.16.6, didnt expect that to be honest.

https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=50646&view=logs&j=4b4d380c-92b5-58cf-5b80-2c6c9b1d5805&t=875b853d-e31b-5e7e-0c21-cef3db7ce4e5&l=123
> > Right and it works for you but what's the numpy version on the failing CI task
> 
> 
> 
> Yup, you are probably right, we are using version 1.16.6, didnt expect that to be honest.
> 
> 
> 
> https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=50646&view=logs&j=4b4d380c-92b5-58cf-5b80-2c6c9b1d5805&t=875b853d-e31b-5e7e-0c21-cef3db7ce4e5&l=123

Ok! You can use np_version_under_1p17 from pandas.compat.numpy to check for that and set the alias to be Any (or stricter if possible) in that case
> Ok! You can use np_version_under_1p17 from pandas.compat.numpy to check for that and set the alias to be Any (or stricter if possible) in that case

Thanks, didn't know about `np._version_under_1p17`. To be strict we can do:

```python
if np_version_under1p17:
    RandomState = Union[int, ArrayLike, np.random.RandomState]
else:
    RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]
```
But not sure if this is desirable design wise. Else  I think the only option left is to use `typing.Any`





> > Ok! You can use np_version_under_1p17 from pandas.compat.numpy to check for that and set the alias to be Any (or stricter if possible) in that case
> 
> Thanks, didn't know about `np._version_under_1p17`. To be strict we can do:
> 
> ```python
> if np_version_under1p17:
>     RandomState = Union[int, ArrayLike, np.random.RandomState]
> else:
>     RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]
> ```
> 
> But not sure if this is desirable design wise. Else I think the only option left is to use `typing.Any`

I don't have an opinion here - I'm not sure how much we care about making type checking backward compatible with old versions of numpy. I'd say for now push either fix to get to green and @jreback can advise since o/w this PR is good to go
@jreback @arw2019 the failing CI seems unrelated, let me know if anything else has to be done for this PR.
Are you sure?
```
pandas/_typing.py:169: error: Cannot assign multiple types to name "RandomState" without an explicit "Type[...]" annotation [misc]
```
doesn't seem unrelated. See [here](https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases) for some help on that
> Are you sure?
> 
> ```
> pandas/_typing.py:169: error: Cannot assign multiple types to name "RandomState" without an explicit "Type[...]" annotation [misc]
> ```
> 
> doesn't seem unrelated. See [here](https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases) for some help on that

Apparently this error is raised because I define `RandomState` twice in an if else. When I remove the if else, there is no such exception.

So this will be correct:
```python
RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]
```

While the following will raise the type annotation error.

```python
if np_version_under1p17:
    RandomState = Union[int, ArrayLike, np.random.RandomState]
else:
    RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]
```

@MarcoGorelli  @arw2019 @jreback for now I removed defining the `RandomState` type and set it to `Any`. After moving all numpy versions in the tests >= 1.17 we can add:

```python
RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]
```
you can just define the type conditionally based on numpy version 

do not use Any (u can also remove the Any and open an issue)
> you can just define the type conditionally based on numpy version

Can you check my comment above about: `if np_version_under1p17:`, that does not seem to work. 

> do not use Any (u can also remove the Any and open an issue)

You mean an issue that we still have to set typehint for `RandomState` I assume?

---

edit: opened ticket #38860 and removed `Any` @jreback 
a number of typing error: https://github.com/pandas-dev/pandas/pull/38594/checks?check_run_id=1631115163
Fixing the typing here isn't so easy - there's

https://github.com/pandas-dev/pandas/blob/a97d90ab8b9497a86491167e031af00567c1e42c/pandas/core/generic.py#L5307

which probably wants renaming, because on the right you have `Union[str, ArrayLike]` and you're trying to cast it to `Series`.

Then there's

https://github.com/pandas-dev/pandas/blob/a97d90ab8b9497a86491167e031af00567c1e42c/pandas/core/generic.py#L5321

, where the return type of `Series.fillna` can be `Series` or `None` (depending on the value of `inplace`, but `mypy` doesn't know that). So I think either `fillna` needs an overload so that when `inplace` is `Literal[False]` the return type is `Series`, or we can just use `cast` (cc @simonjayhawkins )
Looks like the overload approach is taken in `reset_index`

https://github.com/pandas-dev/pandas/blob/91595138a33b4763942f0612e6c2bec163e15d51/pandas/core/frame.py#L4841-L4872
> Fixing the typing here isn't so easy - there's
> 
> https://github.com/pandas-dev/pandas/blob/a97d90ab8b9497a86491167e031af00567c1e42c/pandas/core/generic.py#L5307
> 
> which probably wants renaming, because on the right you have `Union[str, ArrayLike]` and you're trying to cast it to `Series`.
> 
> Then there's
> 
> https://github.com/pandas-dev/pandas/blob/a97d90ab8b9497a86491167e031af00567c1e42c/pandas/core/generic.py#L5321
> 
> , where the return type of `Series.fillna` can be `Series` or `None` (depending on the value of `inplace`, but `mypy` doesn't know that). So I think either `fillna` needs an overload so that when `inplace` is `Literal[False]` the return type is `Series`, or we can just use `cast` (cc @simonjayhawkins )

Thanks for having a in-depth look at this. This turned out to be way more complex and time consuming than I thought. Would it be an idea to open a new ticket for overload on `fillna`?
Sounds reasonable to me. If this is ready apart from that, maybe it's OK to leave `weights` untyped here for this PR, and take care of it elsewhere?
@erfannariman if you can merge master and see if can get to green (we can defer the typing on the random if that's causing an issue)
> @erfannariman if you can merge master and see if can get to green (we can defer the typing on the random if that's causing an issue)

Yes sure, will do this weekend, this one is basically ready to go except the typing complexity.
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.
> Thanks for having a in-depth look at this. This turned out to be way more complex and time consuming than I thought. Would it be an idea to open a new ticket for overload on `fillna`?

This is done now, if you merge upstream/master then the typing should be a lot simpler here
@erfannariman can you merge master https://github.com/pandas-dev/pandas/pull/38594#issuecomment-822760699
@erfannariman Thanks for the PR. closing as stale. ping if you want to continue.