Skip to content

Conversation

@kyamagu
Copy link
Contributor

@kyamagu kyamagu commented Oct 31, 2025

Changes:

  • Add type annotations to the effects API
  • Add default values so that we do not return None whereever possible

@kyamagu kyamagu requested a review from Copilot October 31, 2025 06:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances type safety and API robustness in the effects module by adding comprehensive type annotations and default values to prevent None returns.

Key Changes

  • Added type annotations throughout the effects API including return types for all properties and method parameters
  • Replaced None returns with sensible default values (e.g., 0.0 for numeric values, False for booleans)
  • Renamed internal value attribute to descriptor for clarity while maintaining backward compatibility

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kyamagu and others added 2 commits October 31, 2025 15:13
This commit addresses GitHub Copilot review comments by implementing proper
handling of descriptor return values. The key issue was that descriptor.get()
returns descriptor objects (like NumericElement) with a .value attribute when
keys exist, not raw values.

Changes:
- Add _get_value() helper function to extract .value attribute from descriptor
  objects while supporting default values
- Update all numeric properties to use _get_value() for proper type conversion
- Fix enum properties (.blend_mode, .type, .glow_type, etc.) to safely handle
  missing keys with appropriate defaults
- Add Enum import for using Enum.Normal as default blend mode
- Fix trailing whitespace in deprecated value property docstring

All 944 tests pass with 96% coverage on effects.py.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@kyamagu kyamagu merged commit 57656b1 into main Oct 31, 2025
7 checks passed
@kyamagu kyamagu deleted the fix/effect-values branch October 31, 2025 06:34
@kyamagu kyamagu mentioned this pull request Nov 9, 2025
5 tasks
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