Skip to content

Conversation

ShaharNaveh
Copy link
Contributor

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



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?



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