Skip to content

add type hint for core/computation/eval #26992

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

Closed
wants to merge 9 commits into from
37 changes: 26 additions & 11 deletions pandas/core/computation/eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@
"""

import tokenize
from typing import Any, Dict, Iterable, List, Optional, Tuple, Union
import warnings

import numpy as np

from pandas.util._validators import validate_bool_kwarg

import pandas as pd
from pandas.core.computation.engines import _engines
from pandas.core.computation.scope import _ensure_scope

from pandas.io.formats.printing import pprint_thing


def _check_engine(engine):
def _check_engine(engine: Optional[str]) -> str:
"""
Make sure a valid engine is passed.

Expand Down Expand Up @@ -60,7 +64,7 @@ def _check_engine(engine):
return engine


def _check_parser(parser):
def _check_parser(parser: str) -> None:
"""
Make sure a valid parser is passed.

Expand All @@ -80,7 +84,7 @@ def _check_parser(parser):
' {valid}'.format(parser=parser, valid=_parsers.keys()))


def _check_resolvers(resolvers):
def _check_resolvers(resolvers: Iterable) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

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

if resolvers is not None:
for resolver in resolvers:
if not hasattr(resolver, '__getitem__'):
Expand All @@ -89,7 +93,7 @@ def _check_resolvers(resolvers):
'the __getitem__ method'.format(name=name))


def _check_expression(expr):
def _check_expression(expr: str) -> None:
"""
Make sure an expression is not an empty string

Expand All @@ -107,7 +111,7 @@ def _check_expression(expr):
raise ValueError("expr cannot be an empty string")


def _convert_expression(expr):
def _convert_expression(expr: str) -> str:
"""
Convert an object to an expression.

Expand Down Expand Up @@ -136,7 +140,7 @@ def _convert_expression(expr):
return s


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

Choose a reason for hiding this comment

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

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

from pandas.core.computation.expr import tokenize_string

at_top_of_stack = stack_level == 0
Expand All @@ -154,11 +158,20 @@ def _check_for_locals(expr, stack_level, parser):
for toknum, tokval in tokenize_string(expr):
if toknum == tokenize.OP and tokval == '@':
raise SyntaxError(msg)


def eval(expr, parser='pandas', engine=None, truediv=True,
local_dict=None, global_dict=None, resolvers=(), level=0,
target=None, inplace=False):
return None


def eval(expr: str,
parser: str = 'pandas',
engine: str = None,
truediv: bool = True,
local_dict: List[Dict[str, Any]] = None,
global_dict: List[Dict[str, Any]] = None,
resolvers: Tuple = (),
Copy link
Member

Choose a reason for hiding this comment

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

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

level: int = 0,
target: Union[float, np.ndarray, pd.DataFrame, pd.Series] = None,
inplace: bool = False,
) -> Union[float, np.ndarray, pd.DataFrame, pd.Series]:
"""
Evaluate a Python expression as a string using various backends.

Expand Down Expand Up @@ -353,3 +366,5 @@ def eval(expr, parser='pandas', engine=None, truediv=True,
# We want to exclude `inplace=None` as being False.
if inplace is False:
return target if target_modified else ret
else:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't need the else here

return None