add type hint for core/computation/eval #26992

xcz011 · 2019-06-21T20:18:46Z

closes Annotate DataFrame (API exposed items from pandas.core.api) #26792
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

xcz011 · 2019-06-21T20:23:18Z

I am not fully sure about target variable type and return type. If anyone could take a look and give some feedback, that will be great. Thanks

WillAyd

Looks good some comments

WillAyd · 2019-06-21T20:30:34Z

pandas/core/computation/eval.py

+ parser: str = 'pandas',
+ engine: str = None,
+ truediv: bool = True,
+ local_dict: dict = None,


Instead of using dict can you use the generic Dict from the typing module and add subscripts for the key / value types?

Thanks for this feedback. I looked through the where local_dict be used https://github.com/pandas-dev/pandas/blob/master/pandas/core/computation/scope.py#L103, but I cannot find out what kind of dict is this? [str, str] ? Thanks !

Hmm I'm not intimately familiar with this code but from what I gather should probably be Dict[str, Any] to simulate what can be injected into locals() and/or globals()

Thanks! I will update soon !

WillAyd · 2019-06-21T20:33:23Z

pandas/core/computation/eval.py



-def _check_expression(expr):
+def _check_expression(expr: Any) -> None:


Kind of vague from the docstring but looks like the type here should be str (Any should be avoided as it doesn't really offer much in the way of type checking)

WillAyd · 2019-06-21T20:35:35Z

pandas/core/computation/eval.py



-def _convert_expression(expr):
+def _convert_expression(expr: Any) -> str:


I think also string here as well (inferring from actual usage rather than docstring)

WillAyd · 2019-06-21T20:35:55Z

pandas/core/computation/eval.py



-def _check_for_locals(expr, stack_level, parser):
+def _check_for_locals(expr: Any, stack_level: int, parser: str) -> None:


WillAyd · 2019-06-21T20:36:41Z

pandas/core/computation/eval.py

+ global_dict: dict = None,
+ resolvers: Iterable = (),
+ level: int = 0,
+ target: Union[np.ndarry, pd.DataFrame, pd.Series] = None,


Typo on ndarray but this can also return a scalar right?

pep8speaks · 2019-06-22T02:46:08Z

Hello @xcz011! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-07-31 02:58:32 UTC

codecov · 2019-06-22T03:23:55Z

Codecov Report

Merging #26992 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@ Coverage Diff @@ ## master #26992 +/- ## ========================================== - Coverage 91.99% 91.99% -0.01%  ========================================== Files 180 180 Lines 50774 50777 +3 ========================================== - Hits 46711 46710 -1  - Misses 4063 4067 +4

Flag	Coverage Δ
#multiple	`90.63% <100%> (ø)`	⬆️
#single	`41.83% <100%> (-0.08%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/computation/eval.py	`97.16% <100%> (+0.08%)`	⬆️
pandas/io/gbq.py	`88.88% <0%> (-11.12%)`	⬇️
pandas/core/frame.py	`96.89% <0%> (-0.12%)`	⬇️
pandas/util/testing.py	`90.84% <0%> (-0.11%)`	⬇️
pandas/core/dtypes/missing.py	`93.93% <0%> (+0.6%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 171615a...8998a0b. Read the comment docs.

codecov · 2019-06-22T03:23:55Z

Codecov Report

Merging #26992 into master will increase coverage by 0.3%.
The diff coverage is 100%.

@@ Coverage Diff @@ ## master #26992 +/- ## ========================================= + Coverage 91.68% 91.99% +0.3%  ========================================= Files 182 180 -2 Lines 50246 50779 +533 ========================================= + Hits 46069 46713 +644  + Misses 4177 4066 -111

Flag	Coverage Δ
#multiple	`90.63% <100%> (-1.06%)`	⬇️
#single	`41.85% <83.33%> (?)`

Impacted Files	Coverage Δ
pandas/core/computation/eval.py	`97.22% <100%> (+0.13%)`	⬆️
pandas/io/excel/_openpyxl.py	`84.71% <0%> (-3.23%)`	⬇️
pandas/util/_decorators.py	`92.13% <0%> (-3.22%)`	⬇️
pandas/core/dtypes/cast.py	`90.72% <0%> (-2.34%)`	⬇️
pandas/io/formats/printing.py	`86.72% <0%> (-2.11%)`	⬇️
pandas/core/internals/blocks.py	`94.38% <0%> (-1.56%)`	⬇️
pandas/core/indexing.py	`93.48% <0%> (-1.55%)`	⬇️
pandas/core/dtypes/common.py	`96.35% <0%> (-1.44%)`	⬇️
pandas/core/arrays/categorical.py	`95.92% <0%> (-1.14%)`	⬇️
pandas/core/groupby/ops.py	`96% <0%> (-0.92%)`	⬇️
... and 164 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 143bc34...8a1ed2b. Read the comment docs.

WillAyd

Comments on 3/4 CI failures. Not sure about last one regarding metaclass but may be related to python/mypy#730 if you can take a look

WillAyd · 2019-06-22T17:25:03Z

pandas/core/computation/eval.py



-def _check_engine(engine):
+def _check_engine(engine: str) -> str:


engine needs to be Optional[str] since None is valid here (cause of one CI failure)

WillAyd · 2019-06-22T17:27:49Z

pandas/core/computation/eval.py



-def _check_for_locals(expr, stack_level, parser):
+def _check_for_locals(expr: str, stack_level: int, parser: str) -> None:


Can just add return None at the end of the body to appease CI

WillAyd · 2019-06-22T17:38:04Z

pandas/core/computation/eval.py

+ truediv: bool = True,
+ local_dict: Dict[str, Any] = None,
+ global_dict: Dict[str, Any] = None,
+ resolvers: Iterable = (),


I think this needs to be List[Dict] per documentation (may also help CI failure)

WillAyd · 2019-06-24T12:55:05Z

pandas/core/computation/eval.py

 # We want to exclude `inplace=None` as being False.
 if inplace is False:
 return target if target_modified else ret
+ else:


Shouldn't need the else here

WillAyd · 2019-06-24T12:57:46Z

pandas/core/computation/eval.py

+ truediv: bool = True,
+ local_dict: List[Dict[str, Any]] = None,
+ global_dict: List[Dict[str, Any]] = None,
+ resolvers: Tuple = (),


So does this only accept a Tuple of Dict? If so we want to add Tuple[Dict[?, ?], ...] as the annotation (replacing ? with appropriate types) and update the docstring which currently suggests a list is appropriate

WillAyd · 2019-06-24T12:58:52Z

pandas/core/computation/eval.py



-def _check_resolvers(resolvers):
+def _check_resolvers(resolvers: Iterable) -> None:


If it's possible we would want to specify the type of object within container classes, so Iterable[<type>] if it can be inferred here

WillAyd · 2019-07-01T16:47:42Z

Removing from 0.25 milestone - @xcz011 if you can merge master and address comments can keep with this though

…core

xcz011 · 2019-07-31T02:59:30Z

Removing from 0.25 milestone - @xcz011 if you can merge master and address comments can keep with this though

Sorry about leaving this open, I think I will try to finish this soon. Thanks

WillAyd · 2019-08-26T02:49:35Z

Can you merge master? Couldn't see old build failures

jreback · 2019-09-08T16:41:44Z

closing as stale, if you'd like to continue pls ping to reopen

add type hint for core/computation/eva

70c8902

WillAyd requested changes Jun 21, 2019

View reviewed changes

WillAyd added the Typing type annotations, mypy/pyright type checking label Jun 21, 2019

replace Any by str for expr

5554ccb

xcz011 added 2 commits June 21, 2019 22:46

update target and return types

658b245

linting updated

8998a0b

WillAyd added this to the 0.25.0 milestone Jun 22, 2019

update local/global_dict type hint to use generic Dict

f31c616

WillAyd requested changes Jun 22, 2019

View reviewed changes

xcz011 added 3 commits June 23, 2019 22:23

update type hint to fix CI

0263816

Add return for inplace is true

9b07641

update return type for inplace and update type for resolver

1e068b4

WillAyd requested changes Jun 24, 2019

View reviewed changes

WillAyd removed this from the 0.25.0 milestone Jul 1, 2019

Merge remote-tracking branch 'upstream/master' into type_hint_pandas_…

8a1ed2b

…core

jreback closed this Sep 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

add type hint for core/computation/eval #26992

add type hint for core/computation/eval #26992

Uh oh!

xcz011 commented Jun 21, 2019

xcz011 commented Jun 21, 2019

WillAyd left a comment

WillAyd Jun 21, 2019

xcz011 Jun 22, 2019

WillAyd Jun 22, 2019

xcz011 Jun 22, 2019

WillAyd Jun 21, 2019

WillAyd Jun 21, 2019

WillAyd Jun 21, 2019

WillAyd Jun 21, 2019

pep8speaks commented Jun 22, 2019 •

edited

Loading

codecov bot commented Jun 22, 2019

codecov bot commented Jun 22, 2019 •

edited

Loading

WillAyd left a comment

WillAyd Jun 22, 2019

WillAyd Jun 22, 2019

WillAyd Jun 22, 2019

WillAyd Jun 24, 2019

WillAyd Jun 24, 2019

WillAyd Jun 24, 2019

WillAyd commented Jul 1, 2019

xcz011 commented Jul 31, 2019

WillAyd commented Aug 26, 2019

jreback commented Sep 8, 2019 •

edited

Loading

Labels

4 participants



		def _check_expression(expr):
		def _check_expression(expr: Any) -> None:



		def _convert_expression(expr):
		def _convert_expression(expr: Any) -> str:



		def _check_for_locals(expr, stack_level, parser):
		def _check_for_locals(expr: Any, stack_level: int, parser: str) -> None:



		def _check_engine(engine):
		def _check_engine(engine: str) -> str:



		def _check_for_locals(expr, stack_level, parser):
		def _check_for_locals(expr: str, stack_level: int, parser: str) -> None:



		def _check_resolvers(resolvers):
		def _check_resolvers(resolvers: Iterable) -> None:

Uh oh!

add type hint for core/computation/eval #26992

add type hint for core/computation/eval #26992

Uh oh!

Conversation

xcz011 commented Jun 21, 2019

xcz011 commented Jun 21, 2019

WillAyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Jun 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2019-07-31 02:58:32 UTC

codecov bot commented Jun 22, 2019

Codecov Report

codecov bot commented Jun 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

WillAyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Jul 1, 2019

xcz011 commented Jul 31, 2019

WillAyd commented Aug 26, 2019

jreback commented Sep 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Labels

4 participants

pep8speaks commented Jun 22, 2019 •

edited

Loading

codecov bot commented Jun 22, 2019 •

edited

Loading

jreback commented Sep 8, 2019 •

edited

Loading