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

Remove pygame.threads #2762

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

Starbuck5
Copy link
Member

@Starbuck5 Starbuck5 commented Mar 21, 2024

The source file doesn't seem to have had a non-linting related update in the past 7 years, it's not documented, and a scour of Google for mentions saw no one using it. I think this is safe to remove.

Also, unlike when this module was started, the standard library now has a way to do a threaded map, in the form of ThreadPoolExecutor.

@Starbuck5 Starbuck5 requested a review from a team as a code owner March 21, 2024 07:51
@Starbuck5 Starbuck5 added the Code quality/robustness Code quality and resilience to changes label Mar 21, 2024
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

I think we could deprecate it but not remove it just yet.
On a quick search I found atleast one github repo that uses it: https://github.com/pmtkachev/TMNT-Fight-NES/blob/c297f8aa9d802dc3fdbfc4b1b8726ccc95eeff97/src/game_func.py#L2

@Starbuck5
Copy link
Member Author

Starbuck5 commented Mar 22, 2024

I think we could deprecate it but not remove it just yet.
On a quick search I found atleast one github repo that uses it: https://github.com/pmtkachev/TMNT-Fight-NES/blob/c297f8aa9d802dc3fdbfc4b1b8726ccc95eeff97/src/game_func.py#L2

But they're not using the functionality! pygame.threads.Thread is a weird import of threading.Thread. They must've written thread and clicked "fix imports" and picked something from a list. No human would've written that unaided.

This is an undocumented module that hasn't made any progress towards being a real thing since pygame 1.8.1, and has never been documented anywhere. The functionality is available in the standard library now. I say we pull the plug on the inevitable and just remove it.

I don't see an undocumented experimental module that no one is actually using as being covered by any backwards compatibility concerns.

@oddbookworm
Copy link
Member

But they're not using the functionality! pygame.threads.Thread is a weird import of threading.Thread. They must've written thread and clicked "fix imports" and picked something from a list. No human would've written that unaided.

Side note:
I created a pull request there to get rid of the weird alias and just use the real thing lol

Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

LGTM!

test/image_test.py Show resolved Hide resolved
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

Okay sure, let's remove this. Thanks for the PR 🎉

@Starbuck5 Starbuck5 merged commit 987436c into pygame-community:main Mar 28, 2024
29 checks passed
@Starbuck5 Starbuck5 added this to the 2.5.0 milestone Mar 28, 2024
@Starbuck5 Starbuck5 deleted the remove-pygame-threads branch March 28, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality/robustness Code quality and resilience to changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants