ENH: add ignore index to DataFrame / Series.sample

[erfannariman]

• closes ENH: add ignore_index to DataFrame / Series.sample  #38581
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[pep8speaks]

Hello @erfannariman! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-12-31 17:42:29 UTC

[jreback]

looks good small comment. pls merge master (and some checks are failing)

[erfannariman]

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 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:

RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]

While the following will raise the type annotation error.

if np_version_under1p17:
    RandomState = Union[int, ArrayLike, np.random.RandomState]
else:
    RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]

[erfannariman]

@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:

RandomState = Union[int, ArrayLike, np.random.Generator, np.random.RandomState]

[jreback]

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)

[erfannariman]

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

[jreback]

a number of typing error: https://github.com/pandas-dev/pandas/pull/38594/checks?check_run_id=1631115163

[MarcoGorelli]

Fixing the typing here isn't so easy - there's

pandas/pandas/core/generic.py

Line 5307 in a97d90a

weights = pd.Series(weights, dtype="float64")

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

pandas/pandas/core/generic.py

Line 5321 in a97d90a

weights = weights.fillna(0)

, 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 )

[MarcoGorelli]

Looks like the overload approach is taken in reset_index

pandas/pandas/core/frame.py

Lines 4841 to 4872 in 9159513

	@overload
	# https://github.com/python/mypy/issues/6580
	# Overloaded function signatures 1 and 2 overlap with incompatible return types
	def reset_index( # type: ignore[misc]
	self,
	level: Optional[Union[Hashable, Sequence[Hashable]]] = ...,
	drop: bool = ...,
	inplace: Literal[False] = ...,
	col_level: Hashable = ...,
	col_fill: Hashable = ...,
	) -> DataFrame:
	...
	@overload
	def reset_index(
	self,
	level: Optional[Union[Hashable, Sequence[Hashable]]] = ...,
	drop: bool = ...,
	inplace: Literal[True] = ...,
	col_level: Hashable = ...,
	col_fill: Hashable = ...,
	) -> None:
	...
	def reset_index(
	self,
	level: Optional[Union[Hashable, Sequence[Hashable]]] = None,
	drop: bool = False,
	inplace: bool = False,
	col_level: Hashable = 0,
	col_fill: Hashable = "",
	) -> Optional[DataFrame]:

[erfannariman]

Fixing the typing here isn't so easy - there's

pandas/pandas/core/generic.py

Line 5307 in a97d90a

weights = pd.Series(weights, dtype="float64")

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

pandas/pandas/core/generic.py

Line 5321 in a97d90a

weights = weights.fillna(0)

, 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?

[MarcoGorelli]

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?

[jreback]

@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]

@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.

[github-actions]

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.

[MarcoGorelli]

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

[simonjayhawkins]

@erfannariman can you merge master #38594 (comment)

[simonjayhawkins]

@erfannariman Thanks for the PR. closing as stale. ping if you want to continue.