Skip to content

TYP: Annotated pandas/core/indexing.py #29908

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.

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 1 commit into from
Nov 30, 2019

Conversation

ShaharNaveh
Copy link
Member

@ShaharNaveh ShaharNaveh commented Nov 27, 2019

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@gfyoung gfyoung added Indexing Related to indexing on series/frames, not to indexes themselves Typing type annotations, mypy/pyright type checking labels Nov 27, 2019
return True

def _has_valid_positional_setitem_indexer(self, indexer):
def _has_valid_positional_setitem_indexer(self, indexer) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

it looks like this always either raises or returns True; is that accurate? if so, maybe get rid of the return value altogether (might need to update usages)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't feel confident enough in reformatting this function, I'll leave it for someone else who will probably won't break the package.

@@ -2523,7 +2523,7 @@ def _maybe_numeric_slice(df, slice_, include_bool=False):
return slice_


def _can_do_equal_len(labels, value, plane_indexer, lplane_indexer, obj):
def _can_do_equal_len(labels, value, plane_indexer, lplane_indexer, obj) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

can any of the args here be typed?

# select a label or row
return not isinstance(key, slice) and not is_list_like_indexer(key)


def need_slice(obj):
def need_slice(obj) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

seems like obj will be a slice?

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm. I think additional types as proposed by @jbrockmendel would be nice if not too much extra effort

@WillAyd WillAyd added this to the 1.0 milestone Nov 29, 2019
@ShaharNaveh
Copy link
Member Author

lgtm. I think additional types as proposed by @jbrockmendel would be nice if not too much extra effort

The problem is that I don't know how to annotate them:(

@WillAyd WillAyd merged commit f51f098 into pandas-dev:master Nov 30, 2019
@WillAyd
Copy link
Member

WillAyd commented Nov 30, 2019

OK thanks @MomIsBestFriend . Sure can always add more types as you get more familiar with code base

@ShaharNaveh ShaharNaveh deleted the TYP-core-indexing branch December 1, 2019 13:18
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Indexing Related to indexing on series/frames, not to indexes themselves Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants