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

Pyupgrade: Update codebase to Python 3 syntax and features #5810

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

EwoutH
Copy link
Contributor

@EwoutH EwoutH commented Nov 8, 2023

This PR updates the Python codebase to Python 3.0 syntax and features, removing old, deprecated and redundant code. pyupgrade 3.15.0 with --keep-percent-format was used.

The first commit includes upgrading to Python 3.0 syntax and features, including:

  • utf-8 encoding is now the default (PEP 3120, Python 3.0+)
  • Replace list comprehensions by Generator Expressions (PEP 289, Python 2.4+)
  • Replace yield loop by yield from (PEP 380, Python 3.3+)
  • Replace the IOError alias by OSError (PEP 3151, Python 3.3+)
  • Remove the default subclass (object) when defining a class
  • Use New Super syntax (PEP 3135, Python 3.0+)
  • Define sets with curly braces {} instead of set()
  • Remove "r" parameter from open function, which is default
  • Remove forced str("native") literals
  • Remove __future__ imports that are now no longer necessary (the future is here!)

The --keep-percent-format parameter ensures percent-formatted strings are not replaced with .format() strings. This is to keep the amount of changes in this PR limited, and could be done in another PR.

@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 8, 2023

There are currently a few errors like this:

Cython/Plex/Scanners.py:91:22: str objects do not support coercion to unicode, use a unicode string literal instead (u'')

This happens in places where the unicode string literal was removed, like:

-        self.buffer = u''
+        self.buffer = ''

Python 3 treats all strings as Unicode by default, so the u-prefix should be able to be removed in all Python code. This error suggests that Cython is still interpreting the code with Python 2 semantics, expecting an explicit u prefix for Unicode strings, which should not be the case in Python 3.

I have no idea however where or how to resolve this issue. @scoder and/or @da-woods, could you help with this? (if so, feel free to make modifications to this PR/branch)

@da-woods
Copy link
Contributor

da-woods commented Nov 8, 2023

So Cython has a language_level parameter that controls how it reads files. We may well be using 2 internally.

Also - I'd be reluctant to make automated changes to either the tests/ or docs/ directories. They may well be demonstrating something specific. Also the .py files in the docs usually have a matching .pyx file that they want to be in sync with.

@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 8, 2023

Thanks for getting back!

So Cython has a language_level parameter that controls how it reads files. We may well be using 2 internally.

That parameter could be removed, right? Or does is also make a distinction between different Python 3.x minor versions?

Also - I'd be reluctant to make automated changes to either the tests/ or docs/ directories. They may well be demonstrating something specific.

Fair point. Should at least be done in separate commits, so allow CI checking against each other.

I went though the changes, almost all are one of these three:

  • Remove the default subclass (object) when defining a class
  • Remove __future__ imports that are now no longer necessary
  • Define sets with curly braces {} instead of set()

So these are very minor syntax things.

Also the .py files in the docs usually have a matching .pyx file that they want to be in sync with.

What's the proper way to update!

@da-woods
Copy link
Contributor

da-woods commented Nov 8, 2023

So Cython has a language_level parameter that controls how it reads files. We may well be using 2 internally.

That parameter could be removed, right? Or does is also make a distinction between different Python 3.x minor versions?

It could be removed in the sense that we don't need to apply it to Cython itself (it's in setup.py). I think we want to keep the parameter for users though so don't erase all mention of it.

Also the .py files in the docs usually have a matching .pyx file that they want to be in sync with.

What's the proper way to update!

Mostly that we try to keep the vertical spacing the same so when you flick between "pure Python" and "Cython" syntax equivalent lines should be in the same place. So find the matching .pyx file (it should have the same name but a different extension) and edit it so that it matches as well as possible.

@scoder
Copy link
Contributor

scoder commented Nov 8, 2023 via email

@scoder
Copy link
Contributor

scoder commented Nov 8, 2023 via email

Copy link
Contributor

@da-woods da-woods left a comment

Choose a reason for hiding this comment

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

A lot of the changes to the tests look dodgy I think. I think probably removing from __future__ is OK and removing (object) after classes is OK, but a large chunk of them are definitely changing the tests

Cython/Build/Dependencies.py Outdated Show resolved Hide resolved
Cython/Compiler/Lexicon.py Show resolved Hide resolved
Cython/Compiler/Tests/TestCmdLine.py Outdated Show resolved Hide resolved
Cython/Compiler/Tests/TestCmdLine.py Show resolved Hide resolved
Cython/Debugger/DebugWriter.py Outdated Show resolved Hide resolved
tests/run/pep563_annotations.py Outdated Show resolved Hide resolved
tests/run/pyclass_annotations_pep526.py Outdated Show resolved Hide resolved
tests/run/set_discard_remove.py Outdated Show resolved Hide resolved
tests/run/test_grammar.py Outdated Show resolved Hide resolved
tests/run/test_named_expressions.py Outdated Show resolved Hide resolved
@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 9, 2023

@da-woods thanks for the extensive review. Like suggested earlier, I will refrain from changing the tests and (historical) docs, and only update Cython itself.

This commit updates the Cython code to Python 3 syntax and features, removing old, depreciated and redundant code. pyupgrade 3.15.0 with --keep-percent-format was used.

It only updates the code in the Cython directory, tests, docs and other directories aren't updated.

This includes:
- utf-8 encoding is now the default (PEP 3120, Python 3.0+)
- Replace list comprehensions by Generator Expressions (PEP 289, Python 2.4+)
- Replace yield loop by yield from (PEP 380, Python 3.3+)
- Replace the IOError alias by OSError (PEP 3151, Python 3.3+)
- Remove the default subclass (object) when defining a class
- Use New Super syntax (PEP 3135, Python 3.0+)
- Define sets with curly braces {} instead of set()
- Remove "r" parameter from open function, which is default
- Remove forced str("native") literals

The --keep-percent-format parameter ensures percent-formatted strings are not replaced with .format() strings. This could be done in another PR.
@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 9, 2023

A lot of the changes to the tests look dodgy I think.

I reverted all the tests, docs, and everything that's not in the Cython directory. Then I checked all changes, and manually fixed the duplicate try-except cases (I also filed an issue at pyupgrade for this: asottile/pyupgrade#915).

We still need to switch the default from "3str" to "3" in Cython 3.1. The first is only relevant for Python 2 and should just become an alias.

When this is done, please let me know, and I will rebase this PR!

Cython/Build/Dependencies.py Outdated Show resolved Hide resolved
Cython/Build/Dependencies.py Show resolved Hide resolved
Cython/Compiler/Buffer.py Outdated Show resolved Hide resolved
Cython/Compiler/Code.py Outdated Show resolved Hide resolved
Cython/Compiler/ExprNodes.py Outdated Show resolved Hide resolved
Cython/Debugger/libcython.py Outdated Show resolved Hide resolved
Cython/Debugger/libcython.py Outdated Show resolved Hide resolved
Cython/Plex/Regexps.py Outdated Show resolved Hide resolved
Cython/Shadow.py Outdated Show resolved Hide resolved
Cython/Utility/Dataclasses.py Outdated Show resolved Hide resolved
@matusvalo
Copy link
Contributor

matusvalo commented Nov 16, 2023

When this is done, please let me know, and I will rebase this PR!

There are some parts of PR that does not need to wait I think. E.g. removing import fallback:

try:
    from collections.abc import Iterable
except ImportError:
    from collections import Iterable

can be done even now. Does it make sense to move this kind of changes to separate PR?

I have already created PR with removing old compatibility crust: #5824

@EwoutH EwoutH marked this pull request as ready for review November 16, 2023 08:31
@EwoutH
Copy link
Contributor Author

EwoutH commented Nov 16, 2023

I think this PR is actually ready for review/merge.

@scoder scoder added this to the 3.1 milestone Nov 16, 2023
@scoder scoder added the cleanup label Nov 16, 2023
@scoder scoder merged commit f036d94 into cython:master Nov 16, 2023
63 of 64 checks passed
@scoder
Copy link
Contributor

scoder commented Nov 16, 2023

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

4 participants