thingsapi/things.py

Consider introducing constants instead of hard coded string

Closed this issue · 13 comments

Goal

  • As a user of the API
  • I want to use IDE completions
  • so that I can avoid typos and now which values are valid

Suggested approach
In order to achieve this goal I suggest to use constants, ideally with some form of type hinting. E.g. via an enum.

Additional context
For example instead of todos = things.todos(start="Anytime") it would be todos = things.todos(start=StartType.ANYTIME)

class StartType(str, Enum):
    """Valid values for to-do start."""

    INBOX: str = "Inbox"
    ANYTIME: str = "Anytime"
    SOMEDAY: str = "Someday"

Actually, using the @validate_arguments decorator of pydantic together with using some type hinting (e.g. using Literal) works quite nicely.

mikez commented

IDE completions are handy, I agree. I also think Enum and @validate_arguments are useful. However, in this instance, it seems like overengineering to me, because start is already being validated. I vote not to do this.

It's currently being validated because we've implemented validation manually. Using type hinting would allow us to remove such code (as pydantic could do it automatically) and also an IDE with, e.g., a mypy plugin, would indicate such an error while writing code using the API. But all this is not crucial, I agree ;)

mikez commented

@AlexanderWillner I see it as a trade-off. For smaller non-stable projects I find type hinting adds more maintenance costs, more dependancies, and makes code less legible. I modeled the validation after pandas who does validation manually as well. — My intention here is to create a good user experience for the users of the API.

mikez commented

To clarify my bias: I spend a lot of time in non-IDE environments like IPython and the shell.

mikez commented

To clarify: my vote is NO and to close this. Reason: see above (overengineering).

My vote is YES, as imho arbitrary constants should be defined instead of just remembered by the user / documented somewhere (e.g. see PEP 8). Let's see if a third party jumps in at a later stage to pull the needle to one or the other direction ;)

mikez commented

Replacing

import things
things.todos(start="Anytime")

with

import things
from things import StartType

things.todos(start=StartType.ANYTIME)

is overengineering to me.

If you want easier type completion, you could consider parsing the doc string in the IDE. See libraries like pandas and numpy which do it the way we do it.

mikez commented

References:

Ok, ok, ok. I see we come from different backgrounds - as also shown in our discussion about static/automatic type checking ;) I was more thinking in the lines things.todos(start=things.START_ANYTIME)(which would be shorter and also allows for things.todos(start="Anytime") at the same time. But I see/accept that this is not too common in the Python world and I'm willing to follow the Python idiomatic language convention...you never stop learning.

mikez commented

@AlexanderWillner What's your background? I intend not to be dogmatic, but rather to think from first principles. I like whatever makes the library easy to use and the code easy to read for someone who is new to this project.

Mainly statically typed languages, such as Java, my favourite language in this context is rust.

mikez commented

@AlexanderWillner To clarify: I don't believe in reasonings like "coming from different backgrounds" and "Python world". I think there are powerful tools out there like static typing, enum, Python, Rust, etc. They are tools. My intention has been throughout to understand your deeper motive for why you suggest these tools in this specific situation. I'm not opposed to the tools themselves.