Skip to content

Conversation

@dangotbanned
Copy link
Member

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below

Now, we only have 1 case which uses the 2x @overload(s).
I'm keeping the 3x as well for now, just in case that changes πŸ™‚

- Full revert of #2731
- Not needed since #2969

Now, we only have 1 case which uses the 2x `@overload`(s) but I'm keeping the 3x as well for now πŸ™‚
Copy link
Member

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

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

I will never complain on these simplifications. As a general comment, the typing for this function seems a bit hard to maintain and not very scalable πŸ₯²

@dangotbanned
Copy link
Member Author

As a general comment, the typing for this function seems a bit hard to maintain and not very scalable

Couldn't agree more tbf πŸ˜‚

(#2371) was really just trying to add typing to a complex situation we were already in

If we make some progress on (#2486), then we may be able to remove the need for it entirely πŸ™

@dangotbanned dangotbanned merged commit 69df278 into main Sep 19, 2025
29 of 31 checks passed
@dangotbanned dangotbanned deleted the refac-remove-isinstance_or_issubclass-overloads branch September 19, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants