search

PagerDuty / pdpyras

Low-level PagerDuty REST/Events API client for Python
MIT License
130 stars 30 forks source link

Type hint help #129

Closed jstoja closed 9 months ago

jstoja commented 10 months ago

Hello,

Are you interested with help to add types to this project?

Best, Julien

Deconstrained commented 10 months ago

In v5 I added type hints for most methods where I believed it would be the most useful and straightforward, but not everywhere. If you find any function signatures that were excluded, where you believe it would be useful to have type hints, please feel free to add them 😉

Deconstrained commented 10 months ago

Also, is there some other specific change that you had in mind regarding types? (I'm not totally sure I understood you correctly)

jstoja commented 10 months ago

Hi Demitri,

Most functions don't have a return type, which could be useful when using a type checker. I've only used the Events API (v2) recently and for example the documentation mentions rtype: str for method trigger(), but the type isn't specified.

It's absolutely not critical to use the library. If that's not used internally and nobody else requested it that might not be useful for the project (because it adds maintenance effort).

Let me know what you think.

Best, Julien

Deconstrained commented 10 months ago

Thanks; that makes sense, and it doesn't currently have return type hints anywhere. I agree with the idea of baking type expectations into the actual code instead of Sphinx directives for return type. There are plenty of functions that are expected to return a value that is always the same type. If you see any where I added an explicit rtype to the docstring for Sphinx but that don't have a return type hint, please feel free to add.

Deconstrained commented 9 months ago

I added a bunch in v5.2.0 and coverage is very close to c omplete.