TenType / discord-rich-presence

A lightweight and safe package for creating custom rich presences on Discord.
https://pypi.org/project/discord-rich-presence/
MIT License
8 stars 2 forks source link

Typing and export PresenceError #3

Closed Amund211 closed 1 year ago

Amund211 commented 1 year ago

Just upstreaming some more of the changes I've made in my copy of your library (https://github.com/Amund211/prism/commit/358bd76b86efe83ed393b6482587d761ae742fb2), though a bit less invasive this time. Feel free to pick and choose which commits you want or don't want.

Summary of the changes:

Amund211 commented 1 year ago

For the sys.platform thing I tried keeping the variable WINDOWS so we don't have to duplicate the string literal everywhere, but mypy didn't like it. https://github.com/Amund211/prism/commit/70ebb79b3c5cad55379a11046d24aafd72ebc488: https://github.com/Amund211/prism/actions/runs/5037578368/jobs/9034513970 https://github.com/Amund211/prism/commit/d918bea15204557ff692f17f5e05ee10e0b4abab: https://github.com/Amund211/prism/actions/runs/5037594511/jobs/9034540744

TenType commented 1 year ago

Thanks for all the help. Right now I am occupied with work but I'll take a look at it when I get the chance.

Amund211 commented 1 year ago

Separate socket classes might be the cleanest option. We're still left with errors from mypy on windows, but that's just because the logic that chooses the _Socket implementation is way to advanced for mypy to understand. See: https://github.com/Amund211/discord-rich-presence/actions/runs/5171453202

Adding a # type: ignore just gives errors on the unicies because it's not a type error there lmao. # type: ignore [attr-defined,unused-ignore] should work in the next release of mypy, other than that I don't really know how to fix it. Then again, I don't think client code is affected by internal type errors in a library anyway, so it might be just okay to leave it, at least until the next mypy version. If that sounds OK to you you can cherry pick this: https://github.com/Amund211/discord-rich-presence/commit/4e6e497ff6a9e7b83b7670aac22870eb97b847fa.

Also, when adding the action to test the typing on windows I realized that I broke python 3.9 compatibility with the new union syntax, so I changed it to the old one in 0a5623c717d0e2a29220fb6da43866045c6a2b5c (now 48f192f75e4724c3724a1e07674b46cf98ee065b).

:^)

Amund211 commented 1 year ago

Prefer Optional[T] over Union[T, None]

Fair, I'll just change the commit and force push

TenType commented 1 year ago

Thanks! I'll use # type: ignore [attr-defined,unused-ignore] for now then.