[AntonisPylos]

Additional tests for all from_* class methods of Color to close #3491

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds test coverage for Color instance methods that mirror classmethod conversions. Tests verify that calling from_* methods on Color instances returns expected color component tuples, validating existing functionality without introducing API changes.

Changes

Cohort / File(s)	Change Summary
Color instance method tests test/color_test.py	Added tests for instance method calls to from_cmy, from_hsva, from_hsla, from_i1i2i3, from_normalized, and from_hex, each verifying that instance method results match their classmethod counterparts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add more tests for color class methods' accurately reflects the main change of adding test coverage for color class methods.
Description check	✅ Passed	The description appropriately states the purpose of the PR and references the linked issue #3491 that requests additional test coverage.
Linked Issues check	✅ Passed	The changes add tests for all from_* class methods (from_cmy, from_hsva, from_hsla, from_i1i2i3, from_normalized, from_hex) to verify they work when called on instances and produce expected results, fulfilling issue #3491 requirements.
Out of Scope Changes check	✅ Passed	All changes are test-only additions to validate existing functionality; no out-of-scope modifications to core logic or unrelated areas were introduced.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/color_test.py (1)

790-790: Tests validate instance methods correctly, but consider testing with varied initial colors.

The instance method tests work correctly. However, to better align with issue #3491's requirement that "the original value of the instance should not affect the resulting color", consider testing with different initial colors beyond (0, 0, 0, 0). The test_from_hex method demonstrates this pattern well by using varied initial colors like (63, 12, 83) and (52, 31, 8, 255).

Example for test_from_cmy:

 def test_from_cmy(self):
     cmy = pygame.Color.from_cmy(0.5, 0.5, 0.5)
     cmy_tuple = pygame.Color.from_cmy((0.5, 0.5, 0.5))
     cmy_instance = pygame.Color(0, 0, 0, 0).from_cmy(0.5, 0.5, 0.5)
+    cmy_instance2 = pygame.Color(255, 128, 64, 200).from_cmy(0.5, 0.5, 0.5)

     expected_cmy = (127, 127, 127)

     self.assertEqual(expected_cmy, cmy)
     self.assertEqual(expected_cmy, cmy_tuple)
     self.assertEqual(expected_cmy, cmy_instance)
+    self.assertEqual(expected_cmy, cmy_instance2)

Also applies to: 796-796, 808-808, 814-814, 826-826, 832-832, 844-844, 850-850, 860-860, 866-866

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a966d54 and 8de7e98.

📒 Files selected for processing (1)

• test/color_test.py (7 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-01T20:22:31.010Z

Learnt from: Starbuck5
Repo: pygame-community/pygame-ce PR: 3573
File: src_c/_pygame.h:115-126
Timestamp: 2025-09-01T20:22:31.010Z
Learning: In pygame-ce PR #3573, the PG_SURF_BitsPerPixel function implementation uses hardcoded return values of 32 for YUY2/UYVY/YVYU FOURCC formats because it's copying the exact logic from SDL's internal SDL_GetMasksForPixelFormat function. This ensures perfect compatibility between SDL2 and SDL3 behaviors rather than using the technically more accurate values that might be returned by SDL_GetPixelFormatDetails.

Applied to files:

• test/color_test.py

🧬 Code graph analysis (1)

test/color_test.py (1)

buildconfig/stubs/pygame/color.pyi (12)

• Color (12-94)
• from_cmy (54-54)
• from_cmy (57-57)
• from_hsva (60-60)
• from_hsva (63-63)
• from_hsla (66-66)
• from_hsla (69-69)
• from_i1i2i3 (72-72)
• from_i1i2i3 (75-75)
• from_normalized (78-78)
• from_normalized (81-81)
• from_hex (83-83)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: x86_64
• GitHub Check: Pyodide build
• GitHub Check: build (macos-14)
• GitHub Check: build (windows-latest)
• GitHub Check: Debian (Bookworm - 12) [s390x]
• GitHub Check: dev-check
• GitHub Check: x86
• GitHub Check: debug_coverage (ubuntu-24.04, 3.10.17)
• GitHub Check: debug_coverage (ubuntu-24.04, 3.14.0)
• GitHub Check: AMD64
• GitHub Check: msys2 (ucrt64, ucrt-x86_64)
• GitHub Check: Debian (Bookworm - 12) [armv6]
• GitHub Check: Debian (Bookworm - 12) [armv7]
• GitHub Check: Debian (Bookworm - 12) [ppc64le]
• GitHub Check: msys2 (clang64, clang-x86_64)
• GitHub Check: aarch64
• GitHub Check: x86_64
• GitHub Check: build (ubuntu-22.04)
• GitHub Check: i686
• GitHub Check: msys2 (mingw64, x86_64)

🔇 Additional comments (1)

test/color_test.py (1)

878-882: Excellent test coverage for from_hex instance method!

These tests effectively demonstrate that the original instance value doesn't affect the from_hex result by using varied initial colors. This pattern aligns well with the requirements from issue #3491.

Also applies to: 893-896

[ankith26]

LGTM, thanks for the PR 🎉

[aatle]

LGTM, thanks for contributing.