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

Python 3.13. Remove ruff flakiness #614

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Python 3.13. Remove ruff flakiness #614

merged 5 commits into from
Oct 14, 2024

Conversation

fyellin
Copy link
Contributor

@fyellin fyellin commented Oct 10, 2024

Added test for Python 3.13. (Is it time to remove support for 3.8??)

Codegen was giving me occasional bizarre results because of ruff. Fixed by using a different file each time for "ruff" rather than reusing the same file over and over.

Finding and fixing the bug above required me to print out codegen deltas in a more easily understandable way. I'm keeping those changes.

Remove ruff flakiness
Better showing of deltas when codegen fails
@fyellin fyellin requested a review from Korijn as a code owner October 10, 2024 18:38
@fyellin fyellin mentioned this pull request Oct 10, 2024
@fyellin
Copy link
Contributor Author

fyellin commented Oct 10, 2024

Python 3.8 reached end-of-live on 8 October, 2024. I can pull the plug on 3.8 at the same time I add 3.13, if you want.

Copy link
Member

@almarklein almarklein left a comment

Choose a reason for hiding this comment

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

I suspect this has to do with ruff caching. When I looked up the cli arg to disable caching, I noticed that ruff can also run via stdin/stdout, avoiding the need for a temporary file. Here's a minimal working example:

import subprocess
import sys
FormatError = Exception

cmd = [
        sys.executable,
        "-m",
        "ruff",
        "format",
        "--line-length",
        str(88),
        "--no-cache",
        "--stdin-filename",
        "tmp.py"
        
    ]
input = """
foo  = 3         + 1 8
"""

p = subprocess.run(cmd, input=input.encode(), stdout=subprocess.PIPE, stderr=subprocess.PIPE)

if p.returncode:
    stdout = p.stdout.decode(errors="ignore")
    stderr = p.stderr.decode(errors="ignore")
    raise FormatError(
       f"Could not format source ({p.returncode}).\n\nstdout: "
        + stdout
        + "\n\nstderr: "
        + stderr
    )
else:
    result = p.stdout.decode(errors="ignore")

print(result)

I also separeted stdout and stderr, as it can be problematic, as seen in pygfx/pygfx#862

codegen/tests/test_codegen_z.py Show resolved Hide resolved
@almarklein
Copy link
Member

I can pull the plug on 3.8 at the same time I add 3.13, if you want.

Let's do that too :)

Replaced ruff temporary file with stdin/stdout
@fyellin
Copy link
Contributor Author

fyellin commented Oct 11, 2024

Removed 3.8. I suppose we can now use | and |= on dictionaries, and str.removeprefix.

@fyellin
Copy link
Contributor Author

fyellin commented Oct 11, 2024

I was wrong. It requires an administrator to remove the 3.8 requirement. The branch protection rule requiring that the test py38 pass can only be removed by an administrator, and only an administrator can add the requirement for py313. [As a contributor, I can't even see these settings.]

I've undone my 3.8 -> 3.9 changes. If someone else wants to make them, just update ci.yml, pyproject.toml, and the comment in start.rst

Other changes have been made. ruff now uses stdin/stdout and seems to be doing the right thing.

@fyellin
Copy link
Contributor Author

fyellin commented Oct 13, 2024

This is ready for review and merging.

It tests both Python 3.8 and 3.13. It also includes the fixes to get ruff to work through stdin.

almarklein
almarklein previously approved these changes Oct 14, 2024
Copy link
Member

@almarklein almarklein left a comment

Choose a reason for hiding this comment

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

Looking good!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
codegen/utils.py Show resolved Hide resolved
@Korijn Korijn merged commit 8842001 into pygfx:main Oct 14, 2024
21 checks passed
@fyellin fyellin deleted the Python3.13 branch October 15, 2024 23:33
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