Skip to content

Adapt the Python 3.7 AST changes #2870

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

Merged
merged 3 commits into from
Oct 26, 2017

Conversation

Perlence
Copy link
Contributor

@Perlence Perlence commented Oct 25, 2017

Some bytecode tests are expected to fail in Python 3.7.0a1 and 3.7.0a2, because there was a bug that prevented some environ vars from having effect. It was fixed in python/cpython@d7ac061.

Resolves #2818.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 92.244% when pulling fd7bfa3 on Perlence:rewrite-python-37-docstring into 0b540f9 on pytest-dev:master.

@RonnyPfannschmidt
Copy link
Member

i think we need to add python 3.7 to travis to get the needed results

the failures shown are known and will be fixed

assert isinstance(m.body[0], ast.Expr)
assert isinstance(m.body[0].value, ast.Str)
for imp in m.body[1:3]:
if sys.version_info < (3, 7):
Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to understand this, why remove the docstring from the beginning of the AST in the test? If it should not be there, then perhaps we should assert what we expect to be there in >=3.7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Module docstring is now (in 3.7) a part of Module node, it's not in the body. So I check if there's docstring expression in Python prior to 3.7 and then remove it so the following body items have the same indexes on both Pythons. This way, for example, m.body[0:2] is a slice of imports.

I didn't like this approach. So I split this test in 3: one without docstrings that works on both versions, one with docstrings that's skipped prior to 3.7, and one with docstrings for versions after 3.7. This looked quite ugly as well to me, so I reverted to this variant.

Copy link
Member

Choose a reason for hiding this comment

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

Module docstring is now (in 3.7) a part of Module node, it's not in the body. So I check if there's docstring expression in Python prior to 3.7 and then remove it so the following body items have the same indexes on both Pythons. This way, for example, m.body[0:2] is a slice of imports.

@Perlence thanks for the detailed explanation, everything is clear to me now.

@nicoddemus
Copy link
Member

i think we need to add python 3.7 to travis to get the needed results

We already run Python 3.7 on Travis, that's the "nightly" build.

the failures shown are known and will be fixed

Indeed. I would like to understand better the change to the tests first though.

I propose we make a serious effort of promoting the "nightly" build from Allowed Failures to normal mode only when Python 3.7 reaches beta status.

@nicoddemus nicoddemus merged commit 734c435 into pytest-dev:master Oct 26, 2017
@methane
Copy link

methane commented Mar 10, 2018

We're thinking change again in 3.7b3. In this time, docstring will be moved from attribute to special AST node named DocString. See this example:

source = """\
def foo():
    "docstring"
"""

import ast
t = ast.parse(source)
print(ast.dump(t))

3.6: ... FunctionDef(name='foo', args=arguments(...), body=[Expr(value=Str(s='docstring'))], decorator_list=[], returns=None) ...

3.7b2: ... FunctionDef(name='foo', args=arguments(...), body=[], decorator_list=[], returns=None, docstring='docstring') ...

3.7b3?: ... FunctionDef(name='foo', args=arguments(args=[], vararg=None, kwonlyargs=[], kw_defaults=[], kwarg=None, defaults=[]), body=[DocString(s='docstring')], decorator_list=[], returns=None) ...

Of course, ast.getdocstring() works in all versions.
With this change, you can get lineno of docstring from the DocString node (while there is bug for now).

We want feedback from AST users. Please post your comment on this issue or Python-Dev mailing list.

@nicoddemus
Copy link
Member

@methane thanks for the ping!

blueyed added a commit to blueyed/pytest that referenced this pull request Sep 25, 2020
NOTE: fixing/reverting of `doc = getattr(mod, "docstring", None)` was
missed in 3153740 (pytest-dev#4723).

It was added initially in
pytest-dev#2870, but never made it into a
final Python release.
Basically reverts / revisits based on changes introduced in 734c435.
blueyed added a commit to blueyed/pytest that referenced this pull request Sep 25, 2020
NOTE: fixing/reverting of `doc = getattr(mod, "docstring", None)` was
missed in 3153740 (pytest-dev#4723).

It was added initially in
pytest-dev#2870, but never made it into a
final Python release.  Basically reverts / revisits based on changes introduced in 734c435.

Adjusts a7dfc6f (#243).
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.

adapt the python 3.7 ast additions/changes
5 participants