Skip to content

Cleanup Typing in pandas.core.strings #25904

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 8 commits into from
Apr 3, 2019
Merged

Cleanup Typing in pandas.core.strings #25904

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0a83814
Removed pandas.core.strings from mypy blacklist
WillAyd Mar 28, 2019
f612278
Refactored _shared_docs for clearer typing support
WillAyd Mar 28, 2019
590f50c
isort fixup
WillAyd Mar 28, 2019
2dddb6e
Removed unused import
WillAyd Mar 28, 2019
a54c73c
Fixed broken test
WillAyd Mar 28, 2019
f4f4162
Removed inadvertant push
WillAyd Mar 28, 2019
ad70ed2
Merge remote-tracking branch 'upstream/master' into string-typing
WillAyd Mar 30, 2019
33c2278
Added comment for _doc_args variable
WillAyd Mar 30, 2019
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refactored _shared_docs for clearer typing support
  • Loading branch information
@WillAyd
WillAyd committed Mar 28, 2019
commit f6122786fbb5ebe41cf9384ab31baadc06a5416c
74 changes: 38 additions & 36 deletions pandas/core/strings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import re
import textwrap
import warnings
from typing import Dict, Union

import numpy as np

Expand All @@ -28,7 +29,7 @@
"utf-16", "utf-32"
)

_shared_docs = dict()
_shared_docs = dict() # type: Dict[str, str]


def cat_core(list_of_columns, sep):
Expand Down Expand Up @@ -623,13 +624,13 @@ def str_repeat(arr, repeats):
dtype: object
"""
if is_scalar(repeats):
def rep(x):
def scalar_rep(x):
try:
return compat.binary_type.__mul__(x, repeats)
except TypeError:
return compat.text_type.__mul__(x, repeats)

return _na_map(rep, arr)
return _na_map(scalar_rep, arr)
else:

def rep(x, r):
Expand Down Expand Up @@ -2989,33 +2990,34 @@ def rindex(self, sub, start=0, end=None):
3 sWaPcAsE
dtype: object
""")
_shared_docs['lower'] = dict(type='lowercase', method='lower', version='')
_shared_docs['upper'] = dict(type='uppercase', method='upper', version='')
_shared_docs['title'] = dict(type='titlecase', method='title', version='')
_shared_docs['capitalize'] = dict(type='be capitalized',
method='capitalize', version='')
_shared_docs['swapcase'] = dict(type='be swapcased', method='swapcase',
version='')
_shared_docs['casefold'] = dict(type='be casefolded', method='casefold',
version='\n .. versionadded:: 0.25.0\n')
_doc_args = {} # type: Dict[str, Dict[str, str]]
Copy link
Contributor

@jreback jreback Mar 28, 2019

Choose a reason for hiding this comment

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

you renamed this? does this still work?
Copy link
Member Author

@WillAyd WillAyd Mar 28, 2019

Choose a reason for hiding this comment

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

So to be clear I didn't rename this variable I just created a new one to clarify usage and types. _shared_docs still exists, but with str types for both keys and values.

It had mixed usage before because some values were also dicts and were being used to update other strings, so somewhat confusing as to the purpose of the variable and was causing typing to fail since the % operator can't have a dict as an operand
Copy link
Member Author

@WillAyd WillAyd Mar 28, 2019

Choose a reason for hiding this comment

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

Here's a screenshot of the output of lower locally, which is of the items that was moved to the new variable here:

image
Copy link
Contributor

@jreback jreback Mar 29, 2019

Choose a reason for hiding this comment

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

this doesn't make any sense, why did you change the name?
Copy link
Member Author

@WillAyd WillAyd Mar 29, 2019

Choose a reason for hiding this comment

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

To be clear I didn't change the name. _shared_docs still exists but now with just a type of Dict[str, str] so that it only holds docstrings.

Previously the signature was Dict[str, Union[Dict[str, str]]] because it was holding not only docstrings but also dicts to use for substitution in other docstrings, which was rather convoluted and was failing typing.

Instead of mashing all of those types into one _shared_docs variable it is arguably cleaner to have one dict containing docstrings (_shared_docs) and one containing substitution parameters (here _doc_args).
Copy link
Contributor

@jreback jreback Mar 30, 2019

Choose a reason for hiding this comment

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

ok i see what you did. can you put a comment to reflect this fact
_doc_args['lower'] = dict(type='lowercase', method='lower', version='')
_doc_args['upper'] = dict(type='uppercase', method='upper', version='')
_doc_args['title'] = dict(type='titlecase', method='title', version='')
_doc_args['capitalize'] = dict(type='be capitalized', method='capitalize',
version='')
_doc_args['swapcase'] = dict(type='be swapcased', method='swapcase',
version='')
_doc_args['casefold'] = dict(type='be casefolded', method='casefold',
version='\n .. versionadded:: 0.25.0\n')
lower = _noarg_wrapper(lambda x: x.lower(),
docstring=_shared_docs['casemethods'] %
_shared_docs['lower'])
_doc_args['lower'])
upper = _noarg_wrapper(lambda x: x.upper(),
docstring=_shared_docs['casemethods'] %
_shared_docs['upper'])
_doc_args['upper'])
title = _noarg_wrapper(lambda x: x.title(),
docstring=_shared_docs['casemethods'] %
_shared_docs['title'])
_doc_args['title'])
capitalize = _noarg_wrapper(lambda x: x.capitalize(),
docstring=_shared_docs['casemethods'] %
_shared_docs['capitalize'])
_doc_args['capitalize'])
swapcase = _noarg_wrapper(lambda x: x.swapcase(),
docstring=_shared_docs['casemethods'] %
_shared_docs['swapcase'])
_doc_args['swapcase'])
casefold = _noarg_wrapper(lambda x: x.casefold(),
docstring=_shared_docs['casemethods'] %
_shared_docs['casefold'])
_doc_args['casefold'])

_shared_docs['ismethods'] = ("""
Check whether all characters in each string are %(type)s.
Expand Down Expand Up @@ -3157,42 +3159,42 @@ def rindex(self, sub, start=0, end=None):
3 False
dtype: bool
""")
_shared_docs['isalnum'] = dict(type='alphanumeric', method='isalnum')
_shared_docs['isalpha'] = dict(type='alphabetic', method='isalpha')
_shared_docs['isdigit'] = dict(type='digits', method='isdigit')
_shared_docs['isspace'] = dict(type='whitespace', method='isspace')
_shared_docs['islower'] = dict(type='lowercase', method='islower')
_shared_docs['isupper'] = dict(type='uppercase', method='isupper')
_shared_docs['istitle'] = dict(type='titlecase', method='istitle')
_shared_docs['isnumeric'] = dict(type='numeric', method='isnumeric')
_shared_docs['isdecimal'] = dict(type='decimal', method='isdecimal')
_doc_args['isalnum'] = dict(type='alphanumeric', method='isalnum')
_doc_args['isalpha'] = dict(type='alphabetic', method='isalpha')
_doc_args['isdigit'] = dict(type='digits', method='isdigit')
_doc_args['isspace'] = dict(type='whitespace', method='isspace')
_doc_args['islower'] = dict(type='lowercase', method='islower')
_doc_args['isupper'] = dict(type='uppercase', method='isupper')
_doc_args['istitle'] = dict(type='titlecase', method='istitle')
_doc_args['isnumeric'] = dict(type='numeric', method='isnumeric')
_doc_args['isdecimal'] = dict(type='decimal', method='isdecimal')
isalnum = _noarg_wrapper(lambda x: x.isalnum(),
docstring=_shared_docs['ismethods'] %
_shared_docs['isalnum'])
_doc_args['isalnum'])
isalpha = _noarg_wrapper(lambda x: x.isalpha(),
docstring=_shared_docs['ismethods'] %
_shared_docs['isalpha'])
_doc_args['isalpha'])
isdigit = _noarg_wrapper(lambda x: x.isdigit(),
docstring=_shared_docs['ismethods'] %
_shared_docs['isdigit'])
_doc_args['isdigit'])
isspace = _noarg_wrapper(lambda x: x.isspace(),
docstring=_shared_docs['ismethods'] %
_shared_docs['isspace'])
_doc_args['isspace'])
islower = _noarg_wrapper(lambda x: x.islower(),
docstring=_shared_docs['ismethods'] %
_shared_docs['islower'])
_doc_args['islower'])
isupper = _noarg_wrapper(lambda x: x.isupper(),
docstring=_shared_docs['ismethods'] %
_shared_docs['isupper'])
_doc_args['isupper'])
istitle = _noarg_wrapper(lambda x: x.istitle(),
docstring=_shared_docs['ismethods'] %
_shared_docs['istitle'])
_doc_args['istitle'])
isnumeric = _noarg_wrapper(lambda x: x.isnumeric(),
docstring=_shared_docs['ismethods'] %
_shared_docs['isnumeric'])
_doc_args['isnumeric'])
isdecimal = _noarg_wrapper(lambda x: x.isdecimal(),
docstring=_shared_docs['ismethods'] %
_shared_docs['isdecimal'])
_doc_args['isdecimal'])

@classmethod
def _make_accessor(cls, data):
Expand Down