-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
TYP: Typing hints in pandas/io/formats/{css,csvs}.py #30398
TYP: Typing hints in pandas/io/formats/{css,csvs}.py #30398
Conversation
fb88f60
to
b7d395a
Compare
b7d395a
to
c2d3472
Compare
na_rep="", | ||
path_or_buf: Optional[FilePathOrBuffer[str]] = None, | ||
sep: str = ",", | ||
na_rep: str = "", | ||
float_format=None, | ||
cols=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cols=None, | |
cols: Sequence[Hashable] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any suggestions?
mypy's log for 3 diffrent annotations:
cols: Sequence[Hashable] = None
$ mypy pandas
pandas/io/formats/csvs.py:34: error: Incompatible default for argument "cols" (default has type "None", argument has type "Sequence[Hashable]")
pandas/core/generic.py:3171: error: Argument "cols" to "CSVFormatter" has incompatible type "Optional[Sequence[Optional[Hashable]]]"; expected "Sequence[Hashable]"
Found 2 errors in 2 files (checked 816 source files)
cols: Optional[Sequence[Hashable]] = None
$ mypy pandas
pandas/io/formats/csvs.py:128: error: Argument 1 to "list" has incompatible type "Optional[Sequence[Hashable]]"; expected "Iterable[Hashable]"
pandas/io/formats/csvs.py:139: error: Argument 1 to "len" has incompatible type "Optional[Sequence[Hashable]]"; expected "Sized"
pandas/core/generic.py:3171: error: Argument "cols" to "CSVFormatter" has incompatible type "Optional[Sequence[Optional[Hashable]]]"; expected "Optional[Sequence[Hashable]]"
Found 3 errors in 2 files (checked 816 source files)
cols: Optional[Sequence[Optional[Hashable]]] = None
$ mypy pandas
pandas/io/formats/csvs.py:128: error: Argument 1 to "list" has incompatible type "Optional[Sequence[Optional[Hashable]]]"; expected "Iterable[Optional[Hashable]]"
pandas/io/formats/csvs.py:139: error: Argument 1 to "len" has incompatible type "Optional[Sequence[Optional[Hashable]]]"; expected "Sized"
Found 2 errors in 2 files (checked 816 source files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the annotations for columns
might not be consistent in the IO methods. Can you open up a follow up issue for this and just leave unannotated for now? I think might be more difficult so OK to branch off given there's already a good deal in this PR
pandas/io/formats/csvs.py
Outdated
escapechar=None, | ||
decimal=".", | ||
decimal: Optional[str] = ".", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decimal: Optional[str] = ".", | |
decimal: str = ".", |
I don't think this is optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it still be typed as
decimal: Optional[str] = "."
?
mypy's log:
$ mypy pandas
pandas/core/generic.py:3181: error: Argument "decimal" to "CSVFormatter" has incompatible type "Optional[str]"; expected "str"
Found 1 error in 1 file (checked 816 source files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment
pandas/io/formats/csvs.py
Outdated
quoting: Optional[int] = None, | ||
line_terminator: Optional[str] = "\n", | ||
chunksize: Optional[int] = None, | ||
quotechar: Optional[str] = '"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quotechar: Optional[str] = '"', | |
quotechar: str = '"', |
I don't think this is optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it still be typed as
quotechar: Optional[str] = '"'
?
mypy's log:
$ mypy pandas
pandas/io/formats/csvs.py:88: error: Incompatible types in assignment (expression has type "None", variable has type "str")
Found 1 error in 1 file (checked 816 source files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think similar comment as line_terminator
pandas/io/formats/csvs.py
Outdated
date_format=None, | ||
doublequote=True, | ||
quoting: Optional[int] = None, | ||
line_terminator: Optional[str] = "\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line_terminator: Optional[str] = "\n", | |
line_terminator: str = "\n", |
Don't think optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it still be typed as
line_terminator: Optional[str] = "\n"
?
mypy's log:
$ mypy pandas
pandas/core/generic.py:3164: error: Argument "line_terminator" to "CSVFormatter" has incompatible type "Optional[str]"; expected "str"
Found 1 error in 1 file (checked 816 source files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm there is a minor disconnect between to_csv
and the CSVFormatter
class on this. Can you create a follow up issue? Not sure of easiest way to correct
56fd651
to
115d3ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for any of the errors we just have a disconnect between defaults for CSVFormatter and to_csv. These may or may not necessarily be issues but just separate out and if you can open up a follow up issue to address would be ideal
na_rep="", | ||
path_or_buf: Optional[FilePathOrBuffer[str]] = None, | ||
sep: str = ",", | ||
na_rep: str = "", | ||
float_format=None, | ||
cols=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the annotations for columns
might not be consistent in the IO methods. Can you open up a follow up issue for this and just leave unannotated for now? I think might be more difficult so OK to branch off given there's already a good deal in this PR
pandas/io/formats/csvs.py
Outdated
date_format=None, | ||
doublequote=True, | ||
quoting: Optional[int] = None, | ||
line_terminator: Optional[str] = "\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm there is a minor disconnect between to_csv
and the CSVFormatter
class on this. Can you create a follow up issue? Not sure of easiest way to correct
pandas/io/formats/csvs.py
Outdated
quoting: Optional[int] = None, | ||
line_terminator: Optional[str] = "\n", | ||
chunksize: Optional[int] = None, | ||
quotechar: Optional[str] = '"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think similar comment as line_terminator
pandas/io/formats/csvs.py
Outdated
escapechar=None, | ||
decimal=".", | ||
decimal: Optional[str] = ".", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment
115d3ef
to
6167f1a
Compare
@simonjayhawkins care to look? |
index_label: Optional[Union[bool, Hashable, Sequence[Hashable]]] = None, | ||
mode: str = "w", | ||
encoding: Optional[str] = None, | ||
compression: Union[str, Mapping[str, str], None] = "infer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compression: Union[str, Mapping[str, str], None] = "infer", | |
compression: Optional[Union[str, Mapping[str, str]]] = "infer", |
encoding=None, | ||
compression="infer", | ||
quoting=None, | ||
header: Union[bool, Sequence[Hashable]] = True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question for @WillAyd - do we want Sequence[Optional[Hashable]]]
or is this for readability and only add Optional
when mypy complains?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal preference for only when complaining but it isn't strong if you feel otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine with that. (although may need to make exception for public apis)
thanks @MomIsBestFriend |
…ndexing-1row-df * upstream/master: (333 commits) CI: troubleshoot Web_and_Docs failing (pandas-dev#30534) WARN: Ignore NumbaPerformanceWarning in test suite (pandas-dev#30525) DEPR: camelCase in offsets, get_offset (pandas-dev#30340) PERF: implement scalar ops blockwise (pandas-dev#29853) DEPR: Remove Series.compress (pandas-dev#30514) ENH: Add numba engine for rolling apply (pandas-dev#30151) [ENH] Add to_markdown method (pandas-dev#30350) DEPR: Deprecate pandas.np module (pandas-dev#30386) ENH: Add ignore_index for df.drop_duplicates (pandas-dev#30405) BUG: The setting xrot=0 in DataFrame.hist() doesn't work with by and subplots pandas-dev#30288 (pandas-dev#30491) CI: Fix GBQ Tests (pandas-dev#30478) Bug groupby quantile listlike q and int columns (pandas-dev#30485) ENH: Add ignore_index for df.sort_values and series.sort_values (pandas-dev#30402) TYP: Typing hints in pandas/io/formats/{css,csvs}.py (pandas-dev#30398) BUG: raise on non-hashable Index name, closes pandas-dev#29069 (pandas-dev#30335) Replace "foo!r" to "repr(foo)" syntax pandas-dev#29886 (pandas-dev#30502) BUG: preserve EA dtype in transpose (pandas-dev#30091) BLD: add check to prevent tempita name error, clsoes pandas-dev#28836 (pandas-dev#30498) REF/TST: method-specific files for test_append (pandas-dev#30503) marked unused parameters (pandas-dev#30504) ...
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff