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 support #2608

Closed
SnoopJ opened this issue May 9, 2024 · 11 comments · Fixed by #2628
Closed

Python 3.13 support #2608

SnoopJ opened this issue May 9, 2024 · 11 comments · Fixed by #2628
Assignees
Labels
Milestone

Comments

@SnoopJ
Copy link
Contributor

SnoopJ commented May 9, 2024

Requested Feature

Sopel should support Python 3.13 when possible. 3.13 entered beta on 7 May and is now feature complete, so it's a good time to start tracking what's needed for support. 3.13.0 final is scheduled for October 2024, and release candidates are scheduled for late July and early September.

Problems Solved

No response

Alternatives

No response

Notes

Known problems:

@SnoopJ SnoopJ added the Feature label May 9, 2024
@SnoopJ SnoopJ added this to the 8.1.0 milestone May 9, 2024
@SnoopJ SnoopJ self-assigned this May 9, 2024
@SnoopJ
Copy link
Contributor Author

SnoopJ commented May 19, 2024

It may merit a separate issue, but making a note while it's fresh in my mind from PyCon US where the steering council solicited maintainer help:

It would also be helpful to evaluate the impact of free threading on Sopel. Of course we cannot do this until we can run on 3.13 at all, so it's follow-up work.

@dgw
Copy link
Member

dgw commented May 19, 2024

I don't think it should break anything, but what we need to do depends on what kind of help they want. Benchmark comparisons of our package running on GIL-enabled and GIL-disabled builds?

For our own purposes, testing on no-GIL Python would be good to expose any operations that are technically thread-unsafe but blocked by the GIL in current CPython builds. (But I think we already have Locks and such around those places. Probably.)

@SnoopJ
Copy link
Contributor Author

SnoopJ commented May 20, 2024

Mostly I think it's a good stress test for upstream's benefit just to see if any wheels fall off the wagon. I doubt they will, but it's easy enough to give it a go.

I guess it would be nice to do a perf comparison (the build with --disable-gil can still have the GIL turned back on, so it's only one build to juggle), but that does almost certainly require some thought about what we would even measure (and how). I'd say perf eval is well worth splitting into a separate ticket if we end up wanting to see what the impact is

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Aug 26, 2024

RC1 of Python 3.13 is now out: https://www.python.org/downloads/release/python-3130rc1/

I incidentally spoke a little with nedbat today about how coverage.py tests against free-threaded builds. Turns out that the deadsnakes PPA is providing a nogil build of 3.13 (and apparently also providing nightly builds against main?), this is roughly the point where coverage started tracking this build variation: nedbat/coveragepy@29b1bf5

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Oct 7, 2024

3.13 is fully released now

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Oct 7, 2024

Test suite is green for me on 3.13. I did have to disable two requirements in dev-requirements.txt (sqlalchemy[mypy] and types-pkg-resources), but those would only impact mypy.

1387 passed, 8 xfailed, 22 warnings in 76.40s (0:01:16)

21 of those warnings are caused by passing count by position in dice:

sopel/builtins/dice.py: 21 warnings
  /var/lib/gitea/data/gitea-repositories/snoopj/sopel.git/sopel/builtins/dice.py:272: DeprecationWarning: 'count' is passed as positional argument
    arg_str = re.sub(dice_regexp, "%s", arg_str, 0, re.IGNORECASE | re.VERBOSE)

The other one is a SQLAlchemy thing:

test/test_db.py::test_execute                                                                                                                                           
  /var/lib/gitea/data/gitea-repositories/snoopj/sopel.git/sopel/db.py:337: RemovedIn20Warning: Deprecated API features detected! These feature(s) are not compatible wit
h SQLAlchemy 2.0. To prevent incompatible upgrades prior to updating applications, ensure requirements files are pinned to "sqlalchemy<2.0". Set environment variable SQ
LALCHEMY_WARN_20=1 to show all deprecation warnings.  Set environment variable SQLALCHEMY_SILENCE_UBER_WARNING=1 to silence this message. (Background on SQLAlchemy 2.0 
at: https://sqlalche.me/e/b8d9)
    return self.engine.execute(*args, **kwargs)

@dgw
Copy link
Member

dgw commented Oct 7, 2024

I remembered seeing a deprecation related to that re.sub() warning in the Python 3.13 changelog… The PR adding 3.13 to CI should do an audit of all mentioned functions:

  • re:
    • Deprecate passing the optional maxsplit, count, or flags arguments as positional arguments to the module-level split(), sub(), and subn() functions. These parameters will become keyword-only in a future version of Python.

Whoever takes care of migrating to SQLAlchemy 2.0 (for Sopel 9, hopefully) will of course take care of that warning.

You shouldn't have had a types-pkg-resources requirement to disable; it was removed in #2614 (c04ac82). Is your clone up to date with master?

That leaves me just wondering why you had to disable sqlalchemy[mypy]?

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Oct 7, 2024

You shouldn't have had a types-pkg-resources requirement to disable; it was removed in #2614 (c04ac82). Is your clone up to date with master?

Ah, indeed I had forgotten to pull.

The error with sqlalchemy[mypy] is actually resolved by this as well. Re-running the suite now to be sure nothing got stirred up in the commits I was missing.

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Oct 7, 2024

Yup, all good:

1387 passed, 8 xfailed, 22 warnings in 148.33s (0:02:28)

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Oct 7, 2024

Looks like the sqlalchemy[mypy] thing was fixed upstream in 1.4.53 https://docs.sqlalchemy.org/en/20/changelog/changelog_14.html#change-a623aa18a66956a979824d53d56b22a5

@dgw
Copy link
Member

dgw commented Oct 7, 2024

Hmm, https://docs.sqlalchemy.org/en/20/changelog/changelog_14.html#change-1.4.53-mypy in particular.

It wouldn't be ideal to put #2492 in a minor release, since Sopel's DB API needs to change to accommodate some removals in SQLAlchemy itself. We're probably better off pinning mypy<1.11 if we start running into problems, then prioritize SQLA 2.0 + bringing mypy current again soon after releasing Sopel 8.1.

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 a pull request may close this issue.

2 participants