Update some python deps for python>=3.8 compat #251
Conversation
|
Thanks for the PR @yohanboniface. Here are some assorted thoughts:
|
OK, I can have a look :)
Ah, great! Can I help in some way ?
Just to know, why still supporting python 2 ? |
Do you want to do that as part of this PR?
If you are willing, you could try changing the implementation of You may want to use a new library to do the work that messytables was doing, in which case you can choose a library which you think is best. Just share your choice before you start working with it. We don't want to add too many requirements, and the licenses need to be compatible.
Python 3 is still fairly new in Grist, so many documents still run on Python 2 and use formulas that only work in Python 2 and can't be updated automatically. The good news is that this doesn't affect imports, even for documents using Python 2. So importing code and tests only needs to work in Python 3. |
I'd say so :) Is there a way to rerun only the failing tests ? (I'm running tests with |
|
You can set the environment variable Are you getting JS test failures with the changes here? Did all the Python tests pass? |
Ah thanks for the tip :)
Well, I do have some failure anyway locally, and given the number of tests and the time it takes to run the full suite, it's a bit hard to easily know if there are new tests failing. This is why I wanted to only run the failing tests ;)
Ah! I didn't notice the python tests. Are you talking about the Maybe I've missed contribution guide somewhere ? :) |
This seems to do the trick: There are a bunch of failure I'll investigate! |
|
When running in a python 3.6 venv, I have one error failing, this one: The regex does not seem up to date: Which python 3 version is the "standard" to run test without failure ? So I can use it as reference :) |
(Ignore the docstring in that file mentioning Or you can do what that script does more directly:
No, sorry, you haven't missed anything, this is our fault for not making more things public.
pytest almost works, but not quite. There's one function Also the way to use a venv is to first run
Python 3.9. The patch version doesn't seem to matter - I'm running 3.9.2 locally, but if you put |
ABC classes have moved from collections to collections.abc since python 3.3, and retro compat has ended with 3.8
|
Small journey in Grist tests :) Running with python 3.9.13 in branch Running with python 3.10.6: Upgrading Upgrading Patching Updating No more errors, now let's look at failures: Looking at Test expects Lets try to adapt the expected string. It fails again, but on another assertion: Changing the expected string again. Test passes. Same for Will test later to rerun on python 3.9, but I'm expecting that those tests will Looking at Exception type changed since 2.8.1:
Not sure adapting the docstring will be enough, or anything would expect that Now looking at --- generated
+++ usercode.py
@@ -48,4 +48,4 @@
def badSyntax(rec, table):
# for a in b
# 10
- raise SyntaxError("expected ':'\n\nA `SyntaxError` occurs when Python cannot understand your code.\n\nYou wrote a `for` loop but\nforgot to add a colon `:` at the end\n\n", ('usercode', 1, 11, 'for a in b'))
+ raise SyntaxError('invalid syntax\n\nA `SyntaxError` occurs when Python cannot understand your code.\n\nYou wrote a `for` loop but\nforgot to add a colon `:` at the end\n\n', ('usercode', 1, 11, 'for a in b'))Trying to adpat the expected string. Yay, all tests pass! Let's try again in python 3.9, without surprise: Apart from the On thing to notice, is that I only changed tests, not actual code. So for now, I focus on updating libs and updating the test related to For the record, here is the diff I'm not including in this branch: diff --git a/sandbox/grist/test_completion.py b/sandbox/grist/test_completion.py
index cf22235..2ee6c5f 100644
--- a/sandbox/grist/test_completion.py
+++ b/sandbox/grist/test_completion.py
@@ -62,7 +62,7 @@ class TestCompletion(test_engine.EngineTestCase):
# Should have same type as column.
self.assertGreaterEqual(
set(self.engine.autocomplete("value.", "Schools", "lastModifier", self.user)),
- {'value.startswith(', 'value.replace(', 'value.title('}
+ {'value.startswith(', 'value.replace(', 'value.title()'}
)
self.assertGreaterEqual(
set(self.engine.autocomplete("value.", "Schools", "lastModified", self.user)),
@@ -281,7 +281,7 @@ class TestCompletion(test_engine.EngineTestCase):
# Should treat columns as correct types.
self.assertGreaterEqual(
set(self.engine.autocomplete("$firstName.", "Students", "firstName", self.user)),
- {'$firstName.startswith(', '$firstName.replace(', '$firstName.title('}
+ {'$firstName.startswith(', '$firstName.replace(', '$firstName.title()'}
)
self.assertGreaterEqual(
set(self.engine.autocomplete("$birthDate.", "Students", "lastName", self.user)),
@@ -301,13 +301,13 @@ class TestCompletion(test_engine.EngineTestCase):
set(self.engine.autocomplete("$yearFounded.", "Schools", "budget", self.user)),
{
'$yearFounded.denominator', # Only integers have this
- '$yearFounded.bit_length(', # and this
+ '$yearFounded.bit_length()', # and this
'$yearFounded.real'
}
)
self.assertGreaterEqual(
set(self.engine.autocomplete("$budget.", "Schools", "budget", self.user)),
- {'$budget.is_integer(', '$budget.real'} # Only floats have this
+ {'$budget.is_integer()', '$budget.real'} # Only floats have this
)
def test_suggest_follows_references(self):
@@ -320,7 +320,7 @@ class TestCompletion(test_engine.EngineTestCase):
set(self.engine.autocomplete("$school.yearFounded.","Students", "firstName", self.user)),
{
'$school.yearFounded.denominator',
- '$school.yearFounded.bit_length(',
+ '$school.yearFounded.bit_length()',
'$school.yearFounded.real'
}
)
diff --git a/sandbox/grist/test_gencode.py b/sandbox/grist/test_gencode.py
index dff010c..bd7e3e3 100644
--- a/sandbox/grist/test_gencode.py
+++ b/sandbox/grist/test_gencode.py
@@ -73,10 +73,10 @@ class TestGenCode(unittest.TestCase):
if six.PY3:
saved_sample = saved_sample.replace(
"raise SyntaxError('invalid syntax', ('usercode', 1, 11, u'for a in b'))",
- "raise SyntaxError('invalid syntax\\n\\n"
- "A `SyntaxError` occurs when Python cannot understand your code.\\n\\n"
- "You wrote a `for` loop but\\nforgot to add a colon `:` at the end\\n\\n', "
- "('usercode', 1, 11, 'for a in b'))"
+ """raise SyntaxError("expected ':'\\n\\n"""
+ """A `SyntaxError` occurs when Python cannot understand your code.\\n\\n"""
+ """You wrote a `for` loop but\\nforgot to add a colon `:` at the end\\n\\n", """
+ """('usercode', 1, 11, 'for a in b'))"""
)
self.assertEqual(generated, saved_sample, "Generated code doesn't match sample:\n" +
"".join(difflib.unified_diff(generated.splitlines(True),@alexmojaki all the python 3.9 compatible stuff should be pushed, and should be enough for this PR. Not sure how to move forward, which means making those tests run in the same time on python 3.9 and 3.10, and dealing with messytable. |
|
Thanks @yohanboniface, this is perfect. I'll fully review the pushed changes soon, we'll probably be able to merge them as is. Actually upgrading to 3.10 will be more complicated and require some internal discussion, I don't know how soon it'll be possible to go forward with that. |
|
Thanks @alexmojaki Two questions/suggestions, not for this PR obviously:
|
|
Everything looks good with the PR, thanks! I'll pass your questions on to the team. |
Trying to run tests, I've hit some bugs related to my local python version (3.10).
The issue is related to ABC classes moved from collections to collections.abc since
python 3.3, and retro compat has ended with 3.8
I suggest to upgrade those two packages html5lib and sortedcontainers.
It's gonna be harder for the other package hitting the same issue: messytables. This package seems prehistoric (like I am, I've worked too with OKFN long time ago…), it seems no more maintained (messytables is deprecated in favour of tabular-py, which itself seems deprecated in favor of the Frictionless Framework). The package has a lot of old dependencies, including nosetests, which itself is unmaintained.
I can work on clearing this situation, either by updating the Grist code to move over from messytables or by trying to update messytables (hopefully then released), but I'll wait for your advices on this before ;)
Note: after updating those packages and patching messytables, I still have 17 failing tests. Those seems unrelated to my python version, but I may miss something.