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

Fixes for 3.9 #56

Merged
merged 13 commits into from Apr 11, 2020
Merged

Fixes for 3.9 #56

merged 13 commits into from Apr 11, 2020

Conversation

alexmojaki
Copy link
Contributor

Closes #55, although it may be good to investigate further exactly what's going on with keywords and try to find a solution that doesn't involve checking for versions or astroid.

Git is being exhausting as usual, I don't know why it's included all these old commits.

@alexmojaki
Copy link
Contributor Author

Looks like there's a problem between github and travis, but here's the successful build: https://travis-ci.org/github/gristlabs/asttokens/builds/673368162

@dsagal
Copy link
Member

dsagal commented Apr 10, 2020

Hey Alex, any idea why I am getting this test failure when running locally?

$ tox -e py39
GLOB sdist-make: /Users/dmitry/devel/asttokens/setup.py
py39 create: /Users/dmitry/devel/asttokens/.tox/py39
py39 installdeps: .[test]
py39 inst: /Users/dmitry/devel/asttokens/.tox/.tmp/package/1/asttokens-2.0.4.dev16+gfc208c7.zip
py39 installed: astroid==2.3.3,asttokens==2.0.4.dev16+gfc208c7,attrs==19.3.0,lazy-object-proxy==1.4.3,more-itertools==8.2.0,packaging==20.3,pluggy==0.13.1,py==1.8.1,pyparsing==2.4.7,pytest==5.4.1,six==1.14.0,wcwidth==0.1.9,wrapt==1.11.2
py39 run-test-pre: PYTHONHASHSEED='946126650'
py39 run-test: commands[0] | pytest
================================================= test session starts ==================================================
platform darwin -- Python 3.9.0a5, pytest-5.4.1, py-1.8.1, pluggy-0.13.1
cachedir: .tox/py39/.pytest_cache
rootdir: /Users/dmitry/devel/asttokens, inifile: setup.cfg
collected 107 items

tests/test_astroid.py .............s.................................                                            [ 43%]
tests/test_asttokens.py ......                                                                                   [ 49%]
tests/test_line_numbers.py ...                                                                                   [ 52%]
tests/test_mark_tokens.py ...............................F.F.............                                        [ 96%]
tests/test_util.py ....                                                                                          [100%]

======================================================= FAILURES =======================================================
_________________________________________ TestMarkTokens.test_keyword_arg_only _________________________________________

self = <tests.test_mark_tokens.TestMarkTokens testMethod=test_keyword_arg_only>

    def test_keyword_arg_only(self):
      # See https://bitbucket.org/plas/thonny/issues/52/range-marker-fails-with-ridastrip-split
      source = "f(x=1)\ng(a=(x),b=[y])"
      m = self.create_mark_checker(source)
      self.assertEqual(m.view_nodes_at(1, 0),
                       {'Name:f', 'Call:f(x=1)', 'Expr:f(x=1)', 'Module:' + source})
      self.assertEqual(m.view_nodes_at(2, 0),
                       {'Name:g', 'Call:g(a=(x),b=[y])', 'Expr:g(a=(x),b=[y])'})
      self.assertEqual(m.view_nodes_at(2, 11), {'Name:y'})
      if self.is_astroid_test:
        self.assertEqual(m.view_nodes_at(1, 2), {'Keyword:x=1'})
        self.assertEqual(m.view_nodes_at(1, 4), {'Const:1'})
        self.assertEqual(m.view_nodes_at(2, 2), {'Keyword:a=(x)'})
        self.assertEqual(m.view_nodes_at(2, 8), {'Keyword:b=[y]'})
      else:
>       self.assertEqual(m.view_nodes_at(1, 2), {'keyword:x=1'})
E       AssertionError: Items in the second set but not the first:
E       'keyword:x=1'

tests/test_mark_tokens.py:472: AssertionError
________________________________________ TestMarkTokens.test_mark_tokens_simple ________________________________________

self = <tests.test_mark_tokens.TestMarkTokens testMethod=test_mark_tokens_simple>

    def test_mark_tokens_simple(self):
      source = tools.read_fixture('astroid', 'module.py')
      m = self.create_mark_checker(source)

      # Line 14 is: [indent 4] MY_DICT[key] = val
      self.assertEqual(m.view_nodes_at(14, 4), {
        "Name:MY_DICT",
        "Subscript:MY_DICT[key]",
        "Assign:MY_DICT[key] = val"
      })

      # Line 35 is: [indent 12] raise XXXError()
      self.assertEqual(m.view_nodes_at(35, 12), {'Raise:raise XXXError()'})
      self.assertEqual(m.view_nodes_at(35, 18), {'Call:XXXError()', 'Name:XXXError'})

      # Line 53 is: [indent 12] autre = [a for (a, b) in MY_DICT if b]
      self.assertEqual(m.view_nodes_at(53, 20), {'ListComp:[a for (a, b) in MY_DICT if b]'})
      self.assertEqual(m.view_nodes_at(53, 21), {'Name:a'})
      if self.is_astroid_test:
        self.assertEqual(m.view_nodes_at(53, 23), {'Comprehension:for (a, b) in MY_DICT if b'})
      else:
        self.assertEqual(m.view_nodes_at(53, 23), {'comprehension:for (a, b) in MY_DICT if b'})

      # Line 59 is: [indent 12] global_access(local, val=autre)
      self.assertEqual(m.view_node_types_at(59, 12), {'Name', 'Call', 'Expr'})
      self.assertEqual(m.view_nodes_at(59, 26), {'Name:local'})
      if self.is_astroid_test:
        self.assertEqual(m.view_nodes_at(59, 33), {'Keyword:val=autre'})
      else:
>       self.assertEqual(m.view_nodes_at(59, 33), {'keyword:val=autre'})
E       AssertionError: Items in the second set but not the first:
E       'keyword:val=autre'

tests/test_mark_tokens.py:100: AssertionError
=============================================== short test summary info ================================================
FAILED tests/test_mark_tokens.py::TestMarkTokens::test_keyword_arg_only - AssertionError: Items in the second set but...
FAILED tests/test_mark_tokens.py::TestMarkTokens::test_mark_tokens_simple - AssertionError: Items in the second set b...
================================= 2 failed, 104 passed, 1 skipped, 1 warning in 8.74s ==================================
ERROR: InvocationError for command /Users/dmitry/devel/asttokens/.tox/py39/bin/pytest (exited with code 1)
_______________________________________________________ summary ________________________________________________________
ERROR:   py39: commands failed

Same happens if I make a virtual environment with Python 3.9-dev myself and run pytest from there. Using Python 3.9.0a5 I just installed from https://www.python.org/downloads/release/python-390a5/, but don't see how the exact patch version matters.

@alexmojaki
Copy link
Contributor Author

Sorry, no idea.

@dsagal
Copy link
Member

dsagal commented Apr 10, 2020

Does it pass for you locally? Would you mind sharing instructions on how to install the relevant version of Python 3.9? The version I got from https://www.python.org/downloads/release/python-390a5/ doesn't seem to have the keyword changes that need this fix. (It has the Slice changes.)

https://www.python.org/downloads/release/python-390a5/

@alexmojaki
Copy link
Contributor Author

I used pyenv install 3.9-dev which I imagine is based on CPython master but I'm not sure, I've never worked on the bleeding edge before.

python3.9 --version simply says Python 3.9.0a5+.

Copy link
Member

@dsagal dsagal left a comment

Choose a reason for hiding this comment

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

Thank you! I can confirm that it works, but have one suggestion to distinguish the cases based on the node content rather than python version + node type.

first_token = name
# ast.keyword changed in 3.9 https://bugs.python.org/issue40141
# Astroid was not affected
if sys.version_info < (3, 9) or not isinstance(node, ast.AST):
Copy link
Member

Choose a reason for hiding this comment

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

Thank you! I looked a bit closer, and would like to propose this condition instead, which seems more meaningful. I checked that it works:

+    # Until python 3.9, ast keyword nodes didn't have line info. Astroid has lineno of None.
+    if node.arg is not None and getattr(node, 'lineno', None) is None:

(if it holds, apply old logic, else it's no longer needed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! I'm surprised the attribute was actually missing before.

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

Successfully merging this pull request may close these issues.

failures with cpython master
2 participants