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

4.1.0: Test failures with Python 3.8 #498

Closed
hroncok opened this issue Jul 8, 2019 · 6 comments
Closed

4.1.0: Test failures with Python 3.8 #498

hroncok opened this issue Jul 8, 2019 · 6 comments

Comments

@hroncok
Copy link
Contributor

hroncok commented Jul 8, 2019

I see those build failures on Python 3.8.0b2 and Uranium 4.1.0:

_________________________ test_getUsedSettings[data1] __________________________

data = {'code': '"x"', 'variables': ['x']}

    @pytest.mark.parametrize("data", test_getUsedSettings_data)
    def test_getUsedSettings(data):
        function = SettingFunction(data["code"])
        answer = function.getUsedSettingKeys()
>       assert len(answer) == len(data["variables"])
E       assert 0 == 1
E         -0
E         +1

tests/Settings/TestSettingFunction.py:141: AssertionError
_________________________ test_getUsedSettings[data7] __________________________

data = {'code': "sqrt('x')", 'variables': ['sqrt', 'x']}

    @pytest.mark.parametrize("data", test_getUsedSettings_data)
    def test_getUsedSettings(data):
        function = SettingFunction(data["code"])
        answer = function.getUsedSettingKeys()
>       assert len(answer) == len(data["variables"])
E       assert 1 == 2
E         -1
E         +2

The failures were also present in 4.0.0 with older Python 3.8.0 releases.

@Ghostkeeper
Copy link
Collaborator

We're targetting CPython 3.5.2 right now.

I wouldn't expect minor versions of Python to break backwards compatibility. They're usually pretty good on this part. Maybe it's because they are still in beta. In any case, we don't intend to support all versions of all our dependencies. That's why we package CPython with our releases.

In the future we're probably going to upgrade to newer versions of Python, if we need a newer version for some new feature (I love the formatted strings for instance). We'll look to support those new versions then.

@hroncok
Copy link
Contributor Author

hroncok commented Jul 9, 2019

As a Fedora packager of Cura, I unfortunately cannot package CPython 3.5.2 with Cura (note that you should probably look at least at Python 3.5.6). So far CPython 3.7 worked pretty good, but 3.8 fails. If it's because it is beta, than maybe it's a bug in CPython itself that needs to be fixed. Or maybe it is not because it is beta.

Would you at least accept a fix if I figure this out?

@hroncok
Copy link
Contributor Author

hroncok commented Jul 9, 2019

This breakage probably caused by this change in Python:

python/cpython#9445

I have a fix ready, would you accept it, or should I carry it downstream forever?

diff --git a/UM/Settings/SettingFunction.py b/UM/Settings/SettingFunction.py
index 9ca0d30..28739c7 100644
--- a/UM/Settings/SettingFunction.py
+++ b/UM/Settings/SettingFunction.py
@@ -177,10 +177,16 @@ class _SettingExpressionVisitor(ast.NodeVisitor):
             self.values.add(node.id)
             self.keys.add(node.id)
 
+    # This one is used before Python 3.8
     def visit_Str(self, node: ast.AST) -> None:
         if node.s not in self._knownNames and node.s not in dir(builtins):  # type: ignore #AST uses getattr stuff, so ignore type of node.s.
             self.keys.add(node.s)  # type: ignore
 
+    # This one is used on Python 3.8+
+    def visit_Constant(self, node: ast.AST) -> None:
+        if isinstance(node.value, str) and node.value not in self._knownNames and node.value not in dir(builtins):  # type: ignore #AST uses getattr stuff, so ignore type of node.value.
+            self.keys.add(node.value)  # type: ignore
+
     _knownNames = {
         "math",
         "max",

@Ghostkeeper
Copy link
Collaborator

Sure, we'll accept that bugfix.

Ghostkeeper pushed a commit that referenced this issue Jul 10, 2019
The Cura builds target CPython 3.5.2 now so it's not a problem for that, but if we upgrade to CPython 3.8 or higher in the future, or right now for the package managers in Linux, this fixes some function evaluations.

Fixes #498.
@Ghostkeeper
Copy link
Collaborator

See commit here: d7ed5eb

@hroncok
Copy link
Contributor Author

hroncok commented Jul 10, 2019

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants