search

billdeitrick / pypco

A Python client for the Planning Center Online API.
MIT License
39 stars 13 forks source link

There are missing return types. will it be addressed? #48

Closed 07pepa closed 2 years ago

pastorhudson commented 2 years ago

Do you have any info on this? Like what is missing that you'd like to see?

07pepa commented 2 years ago

i do

look here on public methods like get put post etc... look here

there are no return types so pycharm does not help you with programing and other tools like mypy does not work when you are programing or running tests (so will improve tesing for you and users of this library) further more on public method (ones not prefixed by _ )

there are no types on input eather.... even documented it the machine can not read it..

i am willing to fix it

and there is small exaple

def some_method(name:str,age:int,hight:float) -> str:
   return f"hello {name}  U are {age} old you are {hight} high"

Ps this is pedantintic... but it will definetly improve your project

pastorhudson commented 2 years ago

This would be great! I've used type hints on other projects. And anything that makes pycharm better would be fantastic.

Can you work up a PR?

07pepa commented 2 years ago

yes... it may take some time it is lots of stuff to change. idealy i will try to add mypy as well to verify that project is internaly corect

billdeitrick commented 2 years ago

👋 Hi @07pepa! Type hints would be a great addition. Thanks for your willingness to contribute.

Three things I would ask as you move ahead with this:

  1. Let's try to do it in smaller chunks if possible, rather than one big PR that touches every function.
  2. Let's not do anything that would break backwards compatibility (unit tests should provide that safety net).
  3. I don't think you'll need any additional tests for this, but in general all code should have unit tests coverage before it is merged to master.

Thank you!

07pepa commented 2 years ago

https://github.com/billdeitrick/pypco/pull/54

and mypy did discover some discrepeancies so adding it as well but it is virtualy config less and it is free config (that ignores testing to allow it to be more sloppy in types (that we do not need to check as hard

billdeitrick commented 2 years ago

Completed in #54! Thank you!