Skip to content

Update code to Python 3.7 compliance #5125

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
Dec 27, 2021
Merged

Conversation

oke-aditya
Copy link
Contributor

Simply runs pyupgrade with the option --py37-plus

Since Python 3.6 has officially reached end of life from this year. It might be nice to keep our code a bit up to date :)

P.S. My health has recovered. And extremely sorry for all the delays this month.

@facebook-github-bot
Copy link

facebook-github-bot commented Dec 25, 2021

💊 CI failures summary and remediations

As of commit 9e6a4ad (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@oke-aditya
Copy link
Contributor Author

This is the solution to run recursively on all python files. I am sure I will forget this command and won't find it easily again. Hence pasting it here.

pyupgrade `find . -name "*.py" -type f` --py37-plus

Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks @oke-aditya, LGTM! Although I'm wondering why this only pops up for Python >= 3.7. All of the changes also work for Python 3.6 which is evident by the green CI.

@oke-aditya
Copy link
Contributor Author

Then I guess our code was already compliant to python 3.7 and we aren't using any special Features introduced in python 3.7.

@pmeier
Copy link
Collaborator

pmeier commented Dec 27, 2021

Although it looks like we could automate this, #4585 (review) implies that we can't have it as pre-commit hook. This mean, we probably should run pyupgrade regularly to fix stuff like this.

@oke-aditya
Copy link
Contributor Author

oke-aditya commented Dec 27, 2021

Python 3.7 end of life is in 2023 so probably we don't need to run this soon to upgrade python to 3.8.

But we would need to run it remove code which carries old python syntax.
I guess we can run some cronjob once in 2 months ? Which just reports if such old code exist? Or just leave it as it as. I think doesn't affect much.

@datumbox datumbox merged commit 922db30 into pytorch:main Dec 27, 2021
@github-actions
Copy link

Hey @datumbox!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@oke-aditya oke-aditya deleted the up_py37 branch December 27, 2021 09:17
facebook-github-bot pushed a commit that referenced this pull request Dec 29, 2021
Summary:
* Upgrade to py37

* Update

Reviewed By: prabhat00155

Differential Revision: D33351109

fbshipit-source-id: 236eead0c67aabdb5cceea66a0875fcc7812f135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants