Skip to content

Add typing annotation to assert_index_equal #26535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 8, 2019
Merged

Add typing annotation to assert_index_equal #26535

Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
453875e
Add typing annotation to assert_index_equal
makbigc May 27, 2019
8c70a15
Add annotation to the parameter check_categorical
makbigc May 28, 2019
f176ef5
merge
makbigc May 29, 2019
411dd51
Add None to the typing annotation of assert_index_equal
makbigc May 30, 2019
fe9f8e8
Cast left and right Index as MultiIndex and set None optional
makbigc Jun 3, 2019
571e64f
isort pandas/util/testing.py
makbigc Jun 3, 2019
904a7f2
Move the casting statement up
makbigc Jun 4, 2019
7de3444
None as the return type of assert_index_equal
makbigc Jun 4, 2019
cf96e22
Fix linting error
makbigc Jun 4, 2019
8418a07
merging
makbigc Jun 6, 2019
de3fbf2
merging 2nd
makbigc Jun 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions pandas/util/testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import string
import tempfile
import traceback
from typing import cast, Union, Optional
import warnings
import zipfile

Expand Down Expand Up @@ -515,9 +516,14 @@ def equalContents(arr1, arr2):
return frozenset(arr1) == frozenset(arr2)


def assert_index_equal(left, right, exact='equiv', check_names=True,
check_less_precise=False, check_exact=True,
check_categorical=True, obj='Index'):
def assert_index_equal(left: Index,
right: Index,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a raw Index, is it possible to add a typing.TypeVar to _typing.py:

AnyIndex = TypeVar('AnyIndex ', pd.Index, pd.RangeIndex, pd.Int64Index, pd.MultiIndex, + other index types...)

and then use that here (I'm not super aqianted with typing, so could be wrong)?

That would avoid the need to adapt the code below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ok as all of these are Index subclasses

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point was that AnyIndex would capture .levels and other attributes that are not set on the base Index.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is OK too. MyPy would still complain about those attributes not existing on Index

exact: Union[bool, str] = 'equiv',
check_names: bool = True,
check_less_precise: Union[bool, int] = False,
check_exact: bool = True,
check_categorical: bool = True,
obj: str = 'Index') -> Optional[None]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can just do None for the return (Optional[None] I don't think makes sense)

"""Check that left and right Index are equal.

Parameters
Expand Down Expand Up @@ -599,6 +605,8 @@ def _get_ilevel_values(index, level):
check_less_precise=check_less_precise,
check_exact=check_exact, obj=lobj)
# get_level_values may change dtype
left = cast(MultiIndex, left)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move these to line 597? That is specifically where the inference of a new type should occur would logically fit better there

right = cast(MultiIndex, right)
_check_types(left.levels[level], right.levels[level], obj=obj)

# skip exact index checking when `check_categorical` is False
Expand Down