Skip to content

Conversation

@GilbertYoder
Copy link
Contributor

@GilbertYoder GilbertYoder commented May 16, 2025

Sorry for the massive PR 👀

Summary

The aim of this PR is to improve type hints. #95

Fixing some typing errors as well as adding generics

# this
foo: str = None
bar: dict = {}

# becomes (for example)
foo: str | None = None
bar: dict[str, Any] = {}

I used Pyright for type checking. I commited the pyrightconfig.json so my config can be duplicated.

In most places, I could not deduce the correct generics, so Any is used liberally.
Moving forward these could slowly be made more specific.

Behavioral Changes

I did my best to avoid making behavior changes in 99% of cases.

There are a few cases where I introduced changes that could modify behavior. Here are a few:
paddle_billing/Entities/Notifications/NotificationEvent.py:45 -> raise error
paddle_billing/Notifications/Verifier.py:53 -> raise error

paddle_billing/ResponseParser.py:
Two methods were added: get_list and get_dict.
Both of these are just wrappers for get_data but with the correct type annotations. get_dict will still return a list from get_data if the body is None. This should probably be looked at.

paddle_billing/Notifications/Entities/Entity.py & paddle_billing/Notifications/Entities/Simulations/SimulationEntity.py
Added type(entity_class) is not type.

Open to any feedback also.

Copy link
Contributor

@davidgrayston-paddle davidgrayston-paddle left a comment

Choose a reason for hiding this comment

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

Hello 👋

Thank you for contributing these improvements to the SDK!

We should be able to accept these changes once the related test failures are resolved. I've left a few suggestions that should address these failures.

@GilbertYoder
Copy link
Contributor Author

Thanks for the suggestion. Pushed some changes to fix that.

@davidgrayston-paddle davidgrayston-paddle merged commit fc19c55 into PaddleHQ:main May 20, 2025
6 checks passed
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