pygame-community / pygame-ce

🐍🎮 pygame - Community Edition is a FOSS Python library for multimedia applications (like games). Built on top of the excellent SDL library.
https://pyga.me
792 stars 125 forks source link

Remove color.set_length() entirely (3393) #1671

Open GalacticEmperor1 opened 1 year ago

GalacticEmperor1 commented 1 year ago

Issue №3393 opened by MyreMylar at 2022-08-14 11:28:46

This function has been deprecated and now produces deprecation warnings when running some of the tests (e.g. color_test.py and pixel_copy_test.py). It's fair to have a test for the deprecation warning while the function still exists, and perhaps some tests of it's functionality (perhaps filtered for warnings) - but we should probably try to excise it from unrelated tests at least.

set_length() didn't actually do much, it didn't even clear the data from channels, just blocked access when using index based access so probably it can just be deleted in many places, but some care and attention will be needed.


Comments

*gresm commented at 2022-08-17 13:53:21*

I'm interested on working at this issue.


*B-Salinas commented at 2022-11-14 06:05:37*

@gresm is this still being worked on? I'd love to jump in, new to open-source


*gresm commented at 2022-11-14 08:30:17*

@B-Salinas It somewhat is.

MyreMylar commented 11 months ago

It is probably safe to remove this entirely at this point in 2.4.0. It has been a year since I posted this issue about the deprecation warnings, and seven months since the release of 2.1.3.

JorasOliveira commented 10 months ago

Hello, I am interested in working on this as my first PR, is there anyone already working on this?

ankith26 commented 10 months ago

To my knowledge, no one is working on it, so feel free to take on the issue

JorasOliveira commented 10 months ago

will do, by monday i'll have given a more indepth look as this. Just to know if I have a proper understanding of the task, I should remove the function itself + all mentions/calls to it in the code, then make sure it compiles and works properly, am I missing something?

ankith26 commented 10 months ago

Yes, basically, everywhere it is mentioned in the codebase. The C implementation, unit tests, documentation and type stubs for it must be removed. Do ask out if you get stuck anywhere, I'll be happy to elaborate

Starbuck5 commented 10 months ago

Are we actually ready to remove this function? It's a compat break to do so, and it hasn't been deprecated that long, all things considered. I believe Python 3.7, our minimum version, doesn't even show deprecation warnings by default.

@JorasOliveira if you'd like to contribute to pygame-ce, I recommend joining the pygame community discord and checking in the contributing channel.

Perhaps you'd like to take a look at this webcam issue? https://github.com/pygame-community/pygame-ce/issues/2458

Also as a new contributor it's easier to start out with documentation or examples enhancements (there doesn't need to be an issue)-- those are often very valuable contributions.

JorasOliveira commented 10 months ago

Ok, I'll take a look at issue #2458. I'm actually taking a college elective on open source community's, and need to contribute to something that is not documentation. I'll also swing by the discord this week to take a peek.