Evaluating arithmetic expressions and plotting graphs

Question

I just got started with Python. I created some code, and I want to know what more experienced devs think about it. What can I do better? What to avoid?

#!/usr/bin/env python
# -*- coding: utf-8 -*-
"""
Graphs - Calculate expression or draws a graph from given equation.
This is a conversion of my old program created at college at 2002y. 
Just to learn python at 2020y.
"""
import re
import operator
import math 
import matplotlib.pyplot as plt
DEC_PLACES = 3 #number of decimal places after rounding
FUNCTIONS = {
 'sin': lambda x:math.sin(math.radians(x)),
 'cos': lambda x:math.cos(math.radians(x)),
 'tan': lambda x:math.tan(math.radians(x)),
 'ln': lambda x:math.log(x),
}
OPERS = {
 '+': operator.add,
 '-': operator.sub,
 '*': operator.mul,
 '/': operator.truediv,
 '^': operator.pow, 
}
OP_PRIO = {
 '(':0,
 '+':1,
 '-':1,
 ')':1,
 '*':2,
 '/':2,
 '^':3,
}
NUM_MATCH = re.compile(
'(?:[1-9][0-9]*|0)'
'(?:[.][0-9]+)?'
)
FUN_MATCH = re.compile(
'(?:[a-z]{2,}[(])'
)
def checkBrackets(sFun):
 """
 Function checks brackets in string
 i: string with function
 r: 0 -> brackets failure / 1 -> brackets ok 
 """
 wynik = 0 # int result of scan
 if "(" or ")" in sFun:
 for x in sFun:
 if x == "(":
 wynik += 1
 continue
 elif x == ")":
 wynik -= 1
 continue
 if(wynik != 0): wynik = 0
 else: wynik = 1
 return wynik
def analizeOperations(sFun):
 """
 Function checks if there are two operators one after the other
 i: string with function
 r: true if ok / false when err
 """
 ok = True # returning var
 sFun.replace(" ","")
 for i in range(len(sFun)):
 if sFun[i] in OPERS:
 if i>=1:
 if sFun[i-1] in OPERS:
 #two opers side by side
 ok = False
 break
 return ok 
def analizeOpAfterCB(sFun):
 """
 Function checks if there is operator after closing bracket
 i: string with function
 r: true if ok / false when err
 """
 ok = True # returning var
 sFun.replace(" ","")
 for i in range(len(sFun)):
 if sFun[i] == ")" and (i+1)<len(sFun):
 if sFun[i+1] != ")":
 if not sFun[i+1] in OPERS:
 #missing operator after closing bracket
 ok = False
 break
 return ok 
def toRPN(sFun,x_val):
 """
 Function convert infix string to RPN
 i: string with function infix
 x_val: value for x variable
 r: RPN[] 
 """
 stos = [] #stack
 wyjscie = [] #exit string
 index = 0
 while index < len(sFun):
 expr = sFun[index:]
 is_num = NUM_MATCH.match(expr)
 is_fun = FUN_MATCH.match(expr) 
 if is_num: #if num put on wyjscie
 num = is_num.group(0)
 wyjscie.append(float(num)) 
 index += len(num)
 continue 
 if is_fun: #if function put on stos
 fun = is_fun.group(0)
 fun = fun[:-1] #remove "("
 if fun in FUNCTIONS:
 stos.append(fun)
 index += len(fun)
 continue 
 else:
 raise("Błąd! Nieznana funkcja.")
 if sFun[index] == "(": #if "(" put on stos
 stos.append(sFun[index])
 index += 1
 continue
 if sFun[index] == ")": 
 for i in range(len(stos)-1,0,-1): #if ")" move all operands till "(" to wyjscie LIFO
 if stos[i] == "(":
 del stos[i]
 if stos[i-1] in FUNCTIONS:
 wyjscie.append(stos[i-1])
 del stos[i-1]
 break
 else:
 wyjscie.append(stos[i])
 del stos[i] 
 index += 1
 continue
 if sFun[index].lower() == "x": #insert x value on wyjscie
 wyjscie.append(float(x_val))
 index += 1
 continue 
 if sFun[index] in OPERS: 
 if index == 0: #if this is first char of string insert 0.0 before it
 wyjscie.append(0.0)
 elif sFun[index-1] == "(":
 wyjscie.append(0.0) #if operator is after openning bracket insert 0.0 before it
 if not stos: #if stos is empty insert operator
 stos.append(sFun[index]) 
 index += 1
 continue
 if OP_PRIO[sFun[index]] > OP_PRIO[stos[-1]]: #if oper in sFun has higher prio add it to stos
 stos.append(sFun[index])
 index += 1
 continue 
 else: 
 while len(stos): #if oper in sFun has prio <= oper in stos
 #move all opers from stos to wyjscie with prio >= oper 
 if (OP_PRIO[stos[-1]]>OP_PRIO[sFun[index]]
 or (
 OP_PRIO[stos[-1]] == (OP_PRIO[sFun[index]] 
 and OP_PRIO[sFun[index]]<3)
 )
 ): 
 wyjscie.append(stos[-1])
 del stos[-1]
 else: break
 stos.append(sFun[index])
 index += 1 
 # move stos to wyjscie LIFO
 while len(stos):
 if stos[-1] not in ["(",")",]:
 wyjscie.append(stos[-1])
 del stos[-1]
 return wyjscie
def evalExpr(sFun, x_val = 1):
 """
 Function evaluate RPN string 
 i: string with function infix 
 x_val: value for x variable
 r: value
 """
 stos = [] #stack
 #check string
 if not checkBrackets(sFun):
 raise SyntaxError("The expression have unclosed brackets!")
 elif not analizeOperations(sFun): 
 raise SyntaxError("The expression have incorrectly written operators!")
 elif not analizeOpAfterCB(sFun):
 raise SyntaxError("Missing operator after closing bracket!")
 else:
 sRPN = toRPN(sFun,x_val)
 while len(sRPN): 
 if isinstance(sRPN[0],float):
 stos.append(sRPN[0])
 del sRPN[0]
 continue
 if sRPN[0] in OPERS:
 func = OPERS[sRPN[0]] #get function for oper
 val = func(stos[-2],stos[-1]) 
 del stos[-2:] #remove used vals from stos
 del sRPN[0] 
 stos.append(val)
 continue
 if sRPN[0] in FUNCTIONS:
 func = FUNCTIONS[sRPN[0]] #get function 
 val = func(stos[-1]) 
 del stos[-1] #remove used vals from stos
 del sRPN[0] 
 stos.append(val)
 continue 
 return round(stos[0],DEC_PLACES) #return rounded result
def showHelp():
 print("Allowed operators and functions:")
 print("+-*/^")
 print("sin, cos, tan, ln")
 print("You can enter arithmetic expressions like:")
 print("2*(3-4)^2")
 print("2*sin(30-4*2)")
 print("or functions like:")
 print("2*x^2+3*x+1")
 print("2*sin(x)*x+1")
def main(): 
 expr = input("Enter an arithmetic expression (type help for info):") 
 if expr.lower() == "help":
 showHelp()
 exit()
 if "x" in expr:
 option = input("Expression cotains 'x' variable, enter 'r' for range or 'v' for value:")
 while option.lower() != 'r' and option.lower() != 'v':
 option = input("Expression cotains 'x' variable, enter 'r' for range or 'v' for value:")
 if option == 'v':
 x_val = ''
 while not isinstance(x_val,float):
 try:
 x_val = float(input("Enter x value:"))
 except:
 print("That was no valid number.")
 print("{0} = {1}".format(expr,evalExpr(expr,x_val)))
 else:
 x_val = ''
 x_start = ''
 x_end = ''
 x_step = ''
 while (not isinstance(x_start,float) 
 and not isinstance(x_end,float)
 and not isinstance(x_step,float)
 ):
 try:
 x_start, x_end, x_step = map(float,input("Enter start value, end value and step for range (eg.: 0,5,1): ").split(","))
 except:
 print("That was no valid number.")
 #make a graph
 x = []
 y = []
 #calculating values
 i = x_start
 while i <= x_end:
 x.append(i)
 y.append(evalExpr(expr, i))
 i += x_step
 # plotting the points 
 plt.plot(x, y) 
 # naming the x axis 
 plt.xlabel('x') 
 # naming the y axis 
 plt.ylabel('f(x)') 
 # giving a title to my graph 
 expr += F"\n in range {x_start} to {x_end} step {x_step}"
 plt.title(expr) 
 # function to show the plot 
 plt.show() 
 else:
 print("{0} = {1}".format(expr,evalExpr(expr)))
if __name__ == "__main__":
 main()

• Mast: Welcome to Code Review! Can you provide us more information about the purpose of the code (what made you write it) and in what Python version you wrote it?
• Arek: Thanks! I was using python 3.8. I want to learn python, and I was thinking that coding something is better that just follow some tutorials. That's why I try to write something I was made earlier in c++.

Answer 1

Since you mention that this was originally C++ code, well it shows.

First, style. Python has an official style-guide, PEP8. It recommends writing if condition instead of if(condition) and using lower_case instead of camelCase for variables and functions.

You should also have a look at the standard library and this excellent blog post about better looping. You can also directly return the result of e.g. a boolean expression. Here are how I might write some of your functions:

def check_brackets(s):
 """
 Function checks brackets in string
 s: string with function
 returns: 0 -> brackets failure / 1 -> brackets ok 
 """
 open_brackets = 0
 for c in s:
 if c == "(":
 open_brackets += 1
 elif c == ")":
 if open_brackets:
 open_brackets -= 1
 else:
 return False
 return open_brackets == 0

Note that this will not be fooled by e.g. ")(", in contrast to your code.

from itertools import groupby
def analyze_operations(s):
 """
 Function checks if there are two operators one after the other
 s: string with function
 returns: true if ok / false when err
 """
 s = s.replace(" ","") # need to actually assign it, it is not in-place
 is_oper = map(lambda x: x in OPERS, s)
 return all(len(list(group)) == 1 for x, group in groupby(is_oper) if x)

Note that str.replace is not an in-place operation. So it does not do anything unless you assign the result to a variable. But since this seems to appear in many of your functions, you might want to do that in the calling code and not in every function.

from itertools import tee
def pairwise(iterable):
 "s -> (s0,s1), (s1,s2), (s2, s3), ..."
 a, b = tee(iterable)
 next(b, None)
 return zip(a, b)
def analyze_op_after_CB(s):
 """
 Function checks if there is operator after closing bracket
 s: string with function
 returns: true if ok / false when err
 """
 for c1, c2 in pairwise(s.replace(" ","")):
 if (c1 == ")" 
 and c2 not in OPERS 
 and c2 != ")"):
 return False
 return True

The pairwise function is a recipe from the itertools module.

def eval_expr(s, x_val=1):
 """
 Function evaluate RPN string 
 s: string with function infix 
 x_val: value for x variable
 r: value
 """
 s = s.replace(" ", "")
 if not check_brackets(s):
 raise SyntaxError("The expression have unclosed brackets!")
 elif not analyze_operations(s): 
 raise SyntaxError("The expression have incorrectly written operators!")
 elif not analyze_op_after_CB(s):
 raise SyntaxError("Missing operator after closing bracket!")
 stack = []
 for x in to_RPN(s, x_val):
 if isinstance(x, float):
 stack.append(x)
 elif x in OPERS:
 b, a = stack.pop(), stack.pop()
 stack.append(OPERS[x](a, b))
 elif x in FUNCTIONS:
 stack.append(FUNCTIONS[x](stack.pop()))
 if len(stack) != 1:
 raise SyntaxError("More than one value remains on stack")
 return round(stack[0], DEC_PLACES) #return rounded result

The hardest part is of course to rewrite the conversion to the reverse polish notation, so I will leave that for now :)

• Arek: Thanks for answer. Could you explain why use pairwise instead of for loop?
• Graipher: @Arek Because pairwise does what it says, iterating over pairs. No need for range checking, special casing the end, explicit indexing (which does not work with all iterables).
• Arek: I don't understand what exactly do your version function analyze_operations(s) but it fails with: 12+2*(3*4+10/5)
• Graipher: @Arek: Fixed. It was failing because it determined that there were more than one non-operations in a row (which is fine of course, but I was not filtering for it). Thanks for the other edits!

Answer 2

This looks like a interesting program.

General

It took me a while (a few seconds) to figure out the meaning of "i:" and "r:" in the docstrings, perhaps "parameters" and "returns" are more clear.

function checkBrackets

I do not see the use of the continue's in this case, the if-then statement will finish anyway quickly.

I do not understand the variable name wynik, this will have to do with languages i think seeing raise("Błąd! Nieznana funkcja.") later on.

I think the return value should be a bool and

if(wynik != 0): wynik = 0
else: wynik = 1
return wynik

should be

return wynik == 0

function analizeOperations

This method could be better named checkNoAdjacientOperators

The replace method is not inplace, instead it returns a modified string, so

sFun.replace(" ","")

should be

sFun = sFun.replace(" ","")

If the first character is an operator the functions checks the character with index -1, which results in a check for the last character, this is not what you want, so

for i in range(len(sFun)):

should be

for i in range(1, len(sFun)):

The variable ok can be skipped by changing

 ok = False
 break

with

 return False

and the final return statement with return True (although no multiple returns evangilists might protest).

function analizeOpAfterCB

Same remarks as for method analizeOperations.

function toRPN

too much to handle for me now...

• Andreas Rejbrand: checkNoAdjacientOperators: I prefer the adjacent spelling :)