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

Bug: Import of type "from module import (func1, func2)" can trigger ValueError #246

Open
fuanan opened this issue Aug 30, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@fuanan
Copy link

fuanan commented Aug 30, 2023

Description

Given an import that contains unused function, autoimport exits abnormally.

Steps to reproduce

Given the following code,

from sympy import *
from sympy.parsing.sympy_parser import (parse_expr, standard_transformations, implicit_multiplication_application)
from math import factorial

def fact(n):
    return factorial(n)

def evaluate_expression(expression):
    # replace "!" with the function call to "fact"
    expression = expression.replace('!', ').fact(')
    # add an opening parenthesis before each occurrence of "fact"
    expression = expression.replace('fact', '(fact')
    # replace "√" with the function call to "sqrt"
    expression = expression.replace('√', 'sqrt(') + ')'
    # add an closing parenthesis after each occurrence of "sqrt"
    expression = expression.replace('sqrt', 'sqrt')
    # replace "log10" with the function call to "log"
    expression = expression.replace('log10', 'log(') + ')'
    # add an closing parenthesis after each occurrence of "log"
    expression = expression.replace('log', 'log')

    # parse the expression to sympy and evaluate
    expr = parse_expr(expression, local_dict=namespace, transformations=transformations)

    return expr.evalf()
import unittest

class TestEvaluateExpression(unittest.TestCase):
    def test_simple_expression(self):
        expression = "1+2"
        result = evaluate_expression(expression)
        self.assertEqual(result, 3)

    def test_nested_parentheses(self):
        expression = "(((1+2)*3)^2)"
        result = evaluate_expression(expression)
        self.assertEqual(result, 81)

    def test_exponentiation(self):
        expression = "2^3"
        result = evaluate_expression(expression)
        self.assertEqual(result, 8)

    def test_modulus(self):
        expression = "10%3"
        result = evaluate_expression(expression)
        self.assertEqual(result, 1)

    def test_factorial(self):
        expression = "5!"
        result = evaluate_expression(expression)
        self.assertEqual(result, 120)

    def test_square_root(self):
        expression = "√16"
        result = evaluate_expression(expression)
        self.assertEqual(result, 4)

    def test_logarithm(self):
        expression = "log10(100)"
        result = evaluate_expression(expression)
        self.assertEqual(result, 2)

    def test_complex_expression(self):
        expression = "(((1+2)*3)^2%5)!√4log10(100)"
        result = evaluate_expression(expression)
        self.assertEqual(result, 24)

And we use fix_code to fix it, the autoimport exits with the following traceback:

Traceback (most recent call last):
File "D:\xxx\fix_code_test.py", line 74, in
print(fix_code(code))
File "D:\anaconda3\envs\my_env\lib\site-packages\autoimport\services.py", line 73, in fix_code
return SourceCode(original_source_code, config=config).fix()
File "D:\anaconda3\envs\my_env\lib\site-packages\autoimport\model.py", line 67, in fix
self._fix_flake_import_errors()
File "D:\anaconda3\envs\my_env\lib\site-packages\autoimport\model.py", line 308, in _fix_flake_import_errors
self._remove_unused_imports(import_name)
File "D:\anaconda3\envs\my_env\lib\site-packages\autoimport\model.py", line 478, in _remove_unused_imports
imports.remove(object_name)
ValueError: list.remove(x): x not in list

Plausible cause for this issue:

The _remove_unused_imports function in model.py (lines 441-505) failed to consider the scenario where multiple function names are enclosed in parentheses. Accordingly, given an import like "from module import (func1, func2)", line 476 in _remove_unused_imports will simply separate "(func1, func2)" into "(func1" and "func2)" without removing the parentheses, which trigges an exception of "ValueError: list.remove(x): x not in list" when executing line 478 in _remove_unused_imports.

@fuanan fuanan added the bug Something isn't working label Aug 30, 2023
@lyz-code
Copy link
Owner

Hi @fuanan thanks for taking the time to open the issue. Why do you add parentheses to the import statement? Without them it will work just fine and are less characters to type :S.

from sympy.parsing.sympy_parser import parse_expr, standard_transformations, implicit_multiplication_application

Similar to #245, I don't see why we'd need to support this syntax. But maybe there's a use case that makes total sense.

As it's not a syntax that I use I won't invest time solving it. However if you were to make a PR to solve it that doesn't add much complexity, then I'd be fine merging it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants