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

Add __future__ imports to all pytest modules #2316

Merged
merged 4 commits into from
Mar 17, 2017

Conversation

nicoddemus
Copy link
Member

This prevents silly errors from creeping in Python 2 when testing in Python 3.

Noticed this while working on the "pytest warnings" branch and things failed horribly on Python 2 because the module is named warnings.py. Thought about renaming it, but thought "hey it's 2017 we shouldn't worry about this type of silly error anymore". 😉

While at it:

  • Replaced all py.builtin.print_ calls by native print() calls;
  • Noticed that our CONTRIBUTION could use an update and recommend testing with py36;

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 92.648% when pulling e5021dc on nicoddemus:add-future-imports into 5482dfe on pytest-dev:features.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 92.648% when pulling 4d94707 on nicoddemus:add-future-imports into 5482dfe on pytest-dev:features.

@@ -1,4 +1,4 @@
from __future__ import generators
from __future__ import absolute_import, division, generators, print_function
Copy link
Member

Choose a reason for hiding this comment

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

generators is not needed since python 2.3 - we may just want to remove it already ^^

Copy link
Member Author

Choose a reason for hiding this comment

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

That was my initial reaction and did remove it, but to my surprise that import is actually used:

    def compile(self, filename=None, mode='exec',
                flag=generators.compiler_flag,
                dont_inherit=0, _genframe=None):

I did take a quick look at the docs for an alternative but couldn't find any, so decided to just let it be. Do you have a suggestion?

Copy link
Member

Choose a reason for hiding this comment

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

@nicoddemus that seems to be there exactly to support generators on old python versions, i think it can be disabled - perhaps we can even remove the parameter

Copy link
Member

Choose a reason for hiding this comment

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

@nicoddemus in fact - the place where its used suggests incredibly bad api because any external user would pass wrong flags unless being told about the behavior of the default

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, but I think that refactoring would be out of scope for this PR, don't you agree?

Copy link
Member

Choose a reason for hiding this comment

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

true, we should make an issue about it i think

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

lovely, change, please address the nitpick

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.

3 participants