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

chore: remove Python 3.8, ensure Python 3.13 is included #1332

Merged
merged 5 commits into from
Nov 7, 2024

Conversation

jpivarski
Copy link
Member

No description provided.

@ariostas
Copy link
Collaborator

ariostas commented Nov 7, 2024

There's a bit of clean up that can be done here.

try:
# not available in python 3.8
to_thread = asyncio.to_thread
except AttributeError:
import contextvars
import functools
async def to_thread(func, /, *args, **kwargs):
loop = asyncio.get_running_loop()
ctx = contextvars.copy_context()
func_call = functools.partial(ctx.run, func, *args, **kwargs)
return await loop.run_in_executor(None, func_call)
async def async_wrapper_thread(blocking_func, *args, **kwargs):
if not callable(blocking_func):
raise TypeError("blocking_func must be callable")
# TODO: when python 3.8 is dropped, use `asyncio.to_thread` instead (also remove the try/except block above)
return await to_thread(blocking_func, *args, **kwargs)

I'm not sure if it would be better to do it here or in another PR.

@jpivarski
Copy link
Member Author

If that section of code can be cleaned up because we aren't supporting Python 3.8 anymore, then it makes sense to include it in this PR. I think you have write access to this PR, so go ahead and do what you think should be done. I won't merge it until you say you're done.

@ariostas
Copy link
Collaborator

ariostas commented Nov 7, 2024

Yeah, Python 3.9 introduced asyncio.to_thread, so that chunk of code to support 3.8 can now be dropped. I'll add a commit.

I tried to find other potential cleanups, but I think that was the only one.

@ariostas
Copy link
Collaborator

ariostas commented Nov 7, 2024

I fixed the tests that were failing. I introduced a fix for numpy v1, but I think it's small enough that it should be fine to include in here. It should now be good to go, at least from my side.

@jpivarski
Copy link
Member Author

This looks great; thank you! I can't approve it as a PR because I started it, but I approve anyway. Since you're done, I'll merge it and make Uproot 5.5.0.

(I just checked to see if any tests need to be added to the branch protection—no, thanks to the DAG that ends in a test called "Pass", we can change which tests run and still have them all be required now.)

@jpivarski jpivarski merged commit 95b998b into main Nov 7, 2024
26 checks passed
@jpivarski jpivarski deleted the jpivarski/remove-Python-3.8 branch November 7, 2024 21:04
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.

2 participants