Skip to content
This repository has been archived by the owner on Nov 6, 2019. It is now read-only.

Use print() function in both Python 2 and Python 3 #641

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cclauss
Copy link

@cclauss cclauss commented Jan 23, 2019

Pull Request Template

Thanks for the PR, you probably worked hard on it! Before you submit it for review, please make sure you read our guidelines for contributing to this repository.

Try to make the job easy for your reviewer! Below is a template you can use as a guide for what context your reviewer might need.
If you changed any dev procedures, consider also updating the README.

Description

Legacy print statements are syntax errors in Python 3 but print() function works as expected in both Python 2 and Python 3.

Review Information

Changes

Requirements

@jugglinmike
Copy link
Collaborator

Thanks for the patch, @cclauss. We don't currently support running this project in Python 3, so I'm reluctant to accept changes which suggest otherwise. While Python 3 is definitely a good goal, we currently lack the resources (both in terms of development effort and in terms of tooling) to ensure proper functionality there. Without that, we'll risk silently regressing with every future change. If you're interested in using the project in Python 3, I think the most responsible way to start is by extending our automated testing infrastructure.

It might not be worth the effort, though. That's because we're in the process of reimplementing the services this project provides in various third-party platforms (e.g. TaskCluster and Azure Pipelines). See gh-640, for example.

@cclauss
Copy link
Author

cclauss commented Jan 29, 2019

Thank you for your detailed explanation. These changes will work in both Python 2 and Python 3 so there is no risk in accepting them now. See https://docs.python.org/2/library/functions.html#print Python 2 is no longer supported at the end of this year so most users of web-platform-tests will be wanting to run them on Python 3 or they will search for alternative solutions. https://pythonclock.org

Legacy __print__ statements are syntax errors in Python 3 but __print()__ function works as expected in both Python 2 and Python 3.
@cclauss
Copy link
Author

cclauss commented Jan 29, 2019

Travis CI now runs Python 3 tests in allow_failures mode.

Also https://docs.python.org/3/library/unittest.html#deprecated-aliases

@jugglinmike
Copy link
Collaborator

These changes will work in both Python 2 and Python 3 so there is no risk in accepting them now. See https://docs.python.org/2/library/functions.html#print

The risk of silent regression comes from future changes, not the validity of this patch.

Python 2 is no longer supported at the end of this year so most users of web-platform-tests will be wanting to run them on Python 3 or they will search for alternative solutions. https://pythonclock.org

You are correct about Python 2's end-of-life, but this project is not intended for use by most users of web-platform-tests. The code here is used to fetch browsers and collect results on a regular interval. It uses the infrastructure which is maintained in the web-platform-tests project (largely within the tools directory). That's the code which is of interest to most users of web-platform-tests. While achieving Python 3 compatibility seems tractable for this project, it will be of limited use as long as WPT itself requires Python 2.

They say that if you're not part of the solution, then you're part of the problem. I'm willing to improve things here in advance of WPT, but we need to do that in a way where we can automatically verify correctness of future changes. It's great to see the tests running in CI--that's exactly what I had in mind. However, if it's failing and allowed to fail, then we haven't reached Python 3 compatibility, and we won't know when new commits move us farther from that goal.

And just to be clear: the reason I mentioned re-implementing results collection via third-party services is that once the work is complete, this project will be decommissioned. I'm happy to review and merge a maintainable solution in the mean time--I just want to be sure you're aware of that before putting more time in to this.

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

Successfully merging this pull request may close these issues.

2 participants