TYP use typeddict to define cssdict #40947
Conversation
MarcoGorelli
added
Typing
|
Thanks @attack68 - I've merged master and the tests pass, so looks like they were unrelated |
|
@simonjayhawkins ok here? |
pandas/io/formats/style_render.py
Outdated
| for x in cast(str, style["selector"]).split(",") | ||
| [ | ||
| CSSDict(selector=x, props=style["props"]) | ||
| for x in style["selector"].split(",") |
There was a problem hiding this comment.
I don't think you should need to create the CSSDict explicitly. mypy should check if the expression is a compatible type
return [
{"selector": selector, "props": css_dict["props"]}
for css_dict in styles
for selector in css_dict["selector"].split(",")
]
There was a problem hiding this comment.
just tried that and got
pandas/io/formats/style_render.py:624: error: List comprehension has incompatible type List[Dict[str, Union[str, List[Tuple[str, Union[str, int, float]]]]]]; expected List[CSSDict] [misc]
There was a problem hiding this comment.
it maybe a mypy cache thing.
maybe better then to use for loops and append starting with css_dict : CSSDict = []
the list comp with 4 fors is too difficult to grok.
There was a problem hiding this comment.
hmm, seems ok
(pandas-dev) simon@P340:~/pandas (cssdict)$ mypy pandas
Success: no issues found in 1280 source files
(pandas-dev) simon@P340:~/pandas (cssdict)$ mypy pandas --no-incremental
Success: no issues found in 1280 source files
(pandas-dev) simon@P340:~/pandas (cssdict)$ mypy --version
mypy 0.812
(pandas-dev) simon@P340:~/pandas (cssdict)$
There was a problem hiding this comment.
maybe better then to use for loops and append starting with css_dict : CSSDict = []
should have been css_styles : CSSStyles = []
There was a problem hiding this comment.
Seems like you're right!
Seems like
ret = [
{"selector": selector, "props": css_dict["props"]}
for css_dict in styles
for selector in css_dict["selector"].split(",")
]
return retthrows a mypy error, while
return [
{"selector": selector, "props": css_dict["props"]}
for css_dict in styles
for selector in css_dict["selector"].split(",")
]doesn't - I wasn't expecting them to be different, and I had the former pattern, sorry about that
There was a problem hiding this comment.
yep. mypy will infer the type of the intermediate variable instead of checking the return expression against the return type. could use...
ret: CSSStyles = ...
return ret
and mypy will check the list comp is compatible the variable type declaration
There was a problem hiding this comment.
thanks for explaining/merging! will copy-and-paste next time
| @@ -70,6 +73,8 @@ | |||
| else: | |||
| # typing.final does not exist until py38 | |||
| final = lambda x: x | |||
| # typing.TypedDict does not exist until py38 | |||
| TypedDict = dict | |||
There was a problem hiding this comment.
if we don't need to use the TypedDict at runtime, then we would not need this and we could only use TypedDict in TYPE_CHECKING blocks.
I see the downside being that aliases (and imports of aliases) would also need to be defined in the TYPE_CHECKING block and that we would not be able to construct the TypedDict explicitly in code. but that could be an upside.
no strong preference here yet.
There was a problem hiding this comment.
thinking some more. leave this for now.
from NEP 29, Dec 26, 2021 drop support for Python 3.7
so we should be able to drop python 3.7 on master after the 1.3 release.
|
Thanks @MarcoGorelli |
xref #39942 (comment) - here's a solution, the docs say that