-
Notifications
You must be signed in to change notification settings - Fork 718
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
Testing against Python 3.12 (non-dev) and Python 3.13 (dev) #1079
Conversation
That is... an astonishing number of errors. The pickling errors I can trace back to python/cpython#109462. It looks like setting A lot of the other errors seem to be due to a difference in traceback formatting. An obvious fix is to loosen up a lot of the |
I'm happy to work through these fixes. Since this is non-trivial now, and will take a bit of time, is it something you'd like me to continue with? |
Thanks for the updates! Regarding Python 3.7 being EOL, I still like to keep testing and maintaining this version (with Python 3.5 and 3.6 as well, by the way). It's not much of a burden, so I prefer not to break compatibility with Python versions before a possible 1.0.0 release of Loguru. Sorry about the difficulties you're facing with Python 3.13 and thank you for the preliminary findings. It indeed seems non-trivial. However, I think we should wait for the "beta" version of Python 3.13 before testing it in the CI. Since it's still in "alpha" (see release schedule), breaking changes may be introduced daily, and I don't want this to disrupt Loguru's CI. |
That all sounds very sensible, both with supporting older versions and avoiding CI disruption for newer ones. I'll have a go at fixing this up for Python 3.13, but keep the CI job fail-able. I've got pretty limited time for open source these days, but I get the feeling there's a few small underlying tweaks needed and this will all fall into place. I'll keep the PR draft until then. 🖖 |
Don't feel compelled to do this, but thanks for your help! |
The `Handler.lock` is substituted for a `nullcontext`, keeping Python 3.13 happy. A no-op stub for `Handler.release` is added, keeping all other Python versions happy.
The `Handler.lock` is substituted for a `MockLock`, keeping Python 3.13 happy. A no-op stub for `Handler.release` is added, keeping all other Python versions happy.
This is fixed with a forward-port of `traceback.format_list` from Python 3.5.
The The errors with traceback formatting were... not easily solved 💩. Python 3.13 has thrown a real spanner in the works here. Specifically, in Python 3.13, unless you provide a This throws up some real problems:
This leads me to a horrible conclusion - My solution is to take the original Upsides:
Downsides:
We could mitigate this last downside by forward-porting a Or maybe you know something about the internals of traceback formatting that I don't! This is the best I've got, in any case! 🙇 |
On reflection, I think there's another benefit to taking fully control of traceback formatting. We're already performing at least two passes over this traceback to format it, then colorise it. Although this is a quick way to fix the Python 3.13 problem, it opens up doors to significantly optimize the customized traceback formatting into a nice single-pass efficient render. If you're okay with this approach, I'd like to to forward-port the Python 3.7 traceback formatting to get the recursion handling. But then I think this a good future-proof solution that paves the way for improving and optimizing traceback rendering without the shackles of the stdlib. |
That looks amazing! These are great findings and fixes, thanks a lot for the efforts you've put into this. The custom implementation of Regarding the additional features brought by Python 3.7, I guess we can argue they are not "officially" supported by Loguru since there is no unit tests for them. However, I expect users to complain about regression if Loguru no longer detects repeated frames in traceback. It would be nice to have it, I created #1083. On a side note, Loguru's formatting was originally designed to match the built-in formatting perfectly, with the addition of colors and variables. Today, this goal is no longer possible. It seems difficult to reconcile the built-in's |
Nice! Thanks! Shall I forward-port the Python 3.7 traceback and add the unit tests to close #1083? |
If you're interested in implementing this improvement, your contribution will be very welcome once again. Note that unit tests for exception formatting are generated by uncommenting this line. I mainly created the ticket to keep track of what needs to be done, and to do it eventually before the next release. ;) |
Actually... hrm... although I'd actually love to, my parenting duties are cutting severely into my open source time! So I'm gonna do the sensible dad thing and leave this one to you. 😢 |
Haha, that's definitely the right decision to make. 😄 Thanks again for your work! |
This PR adds Python 3.13 (dev) to the test matrix, and changes the Python 3.12 (dev) entry to 3.12 (non-dev).
Python 3.7 is now EOL, so might be worth dropping from the test matrix. I'll happily provide a follow-up PR if you're interested.