Skip to content
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

Don't misuse KeyError for the custom names function. #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,34 @@ You can also hand the handling of names over to a function, if you prefer:
3

That was a bit of a silly example, but you could use this for pulling values
from a database or file, say, or doing some kind of caching system.
from a database or file, looking up spreadsheet cells, say, or doing some kind of caching system.

The two default names that are provided are ``True`` and ``False``. So if you want to provide your own names, but want ``True`` and ``False`` to keep working, either provide them yourself, or ``.copy()`` and ``.update`` the ``DEFAULT_NAMES``. (See functions example above).
In general, when it attempts to find a variable by name, if it cannot find one,
then it will look in the ``functions`` for a function of that name. If you want your name handler
function to return a "I can't find that name!", then it should raise a ``simpleeval.NameNotDefined``
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function to return a "I can't find that name!", then it should raise a ``simpleeval.NameNotDefined``
function to return an "I can't find that name!", then it should raise a ``simpleeval.NameNotDefined``

exception. Eg:

.. code-block:: python
Copy link
Contributor

Choose a reason for hiding this comment

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

REPL sessions like this should use pycon as the syntax highlighting.

Suggested change
.. code-block:: python
.. code-block:: pycon


>>> def name_handler(node):
... if node.id[0] == 'a':
... return 21
... raise NameNotDefined(node.id[0], "Not found")
...
... simple_eval('a + a', names=name_handler, functions={"b": 100})

42

>>> simple_eval('a + b', names=name_handler, functions={'b': 100})
121

(Note: in that example, putting a number directly into the functions dict was done just to
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(Note: in that example, putting a number directly into the functions dict was done just to
(Note: in that example, putting a number directly into the ``functions`` dict was done just to

show the fall-back to functions. Normally only put actual callables in there.)


The two default names that are provided are ``True`` and ``False``. So if you want to provide
your own names, but want ``True`` and ``False`` to keep working, either provide them yourself,
or ``.copy()`` and ``.update`` the ``DEFAULT_NAMES``. (See functions example above).

Creating an Evaluator Class
---------------------------
Expand Down
22 changes: 14 additions & 8 deletions simpleeval.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,24 +526,30 @@ def _eval_name(self, node):
try:
# This happens at least for slicing
# This is a safe thing to do because it is impossible
# that there is a true exression assigning to none
# that there is a true expression assigning to none
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be open to me running the codebase through codespell and typos?

# (the compiler rejects it, so you can't even
# pass that to ast.parse)
if hasattr(self.names, "__getitem__"):
return self.names[node.id]
if callable(self.names):
return self.names[node.id]

except (TypeError, KeyError):
pass

if callable(self.names):
try:
return self.names(node)
except NameNotDefined:
pass
elif not hasattr(self.names, "__getitem__"):
raise InvalidExpression(
'Trying to use name (variable) "{0}"'
' when no "names" defined for'
" evaluator".format(node.id)
)

except KeyError:
if node.id in self.functions:
return self.functions[node.id]
if node.id in self.functions:
return self.functions[node.id]

raise NameNotDefined(node.id, self.expr)
raise NameNotDefined(node.id, self.expr)

def _eval_subscript(self, node):
container = self._eval(node.value)
Expand Down
30 changes: 30 additions & 0 deletions test_simpleeval.py
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,36 @@ def name_handler(node):
self.t("a", 1)
self.t("a + b", 3)

def test_name_handler_name_not_found(self):
def name_handler(node):
if node.id[0] == "a":
return 21
raise NameNotDefined(node.id[0], "not found")

self.s.names = name_handler
self.s.functions = {"b": lambda: 100}
self.t("a + a", 42)

self.t("b()", 100)

with self.assertRaises(NameNotDefined):
self.t("c", None)

def test_name_handler_raises_error(self):
# What happens if our name-handler raises a different kind of error?
# we want it to ripple up all the way...

def name_handler(_node):
return {}["test"]

self.s.names = name_handler

# This should never be accessed:
self.s.functions = {"c": 42}

with self.assertRaises(KeyError):
self.t("c", None)


class TestWhitespace(DRYTest):
"""test that incorrect whitespace (preceding/trailing) doesn't matter."""
Expand Down