Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support type hints in model fields #1559

Closed
tuukkamustonen opened this issue Apr 4, 2018 · 9 comments
Closed

Support type hints in model fields #1559

tuukkamustonen opened this issue Apr 4, 2018 · 9 comments

Comments

@tuukkamustonen
Copy link

tuukkamustonen commented Apr 4, 2018

Being aware that you don't like type hints (#1298), I'll be bold and ask if you would allow users to annotate their models with type hints?

Problem:

class User(peewee.Model):
    name = CharField()


u = User()
u.name = 'Joe'  # mypy will nag because inferred type is 'CharField'

Cannot:

class User(peewee.Model):
    name = CharField()  # type: str


u = User()
u.name = 'Joe'  # sure works now but
User.select().where(User.name.id == 1)  # violation as User.name is marked as 'str'

Would it be possible to have separate fields for class and instance:

class User(peewee.Model):

    name_c = CharField(instance_field='name')
    name = None  # type: str - simply a placeholder for mypy type
    name: str  # in py3.6+


u = User()
u.name = 'Joe'  # would now have 'str' type
User.select().where(User.name_c.id == 1)  # would have 'CharField' type

The idea being that instance would override the name field with getter/setter (declared as instance_field).

Yeah it is hackish, but it would be better than having to # type: <override> everywhere.

I believe this couldn't go to typeshed stubs anyway due to being context-specific.

@coleifer
Copy link
Owner

coleifer commented Apr 4, 2018

I think type hints are misguided and unpythonic, and it's my stance that they will never be supported by peewee. Python is a high-level dynamic language, peewee plays to these strengths. You couldn't implement peewee in go, it'd look completely different.

I enjoy statically typed languages. When I want static typing I use c or go. When I want dynamic or metaprogramming I use python.

These are my opinions and I understand that others have their own views. My goals for peewee are to make it composable and consistent, and I feel I can do that best by using python's dynamism.

@coleifer coleifer closed this as completed Apr 4, 2018
@tuukkamustonen
Copy link
Author

tuukkamustonen commented Apr 4, 2018

I understand your clause fully, just note that type hints could be supported where dynamic python magic is not needed. Plus in this case the library itself wouldn't imbue type hints - it would just support users declaring them. That would require changes (ie. something like what I suggested above) and I understand that you're reluctant to add that. Anyway, if you (or anyone here) comes up with a simpler idea, chime in.

Maybe I'll ask about this on mypy tracker as well. I'll ping back here if there's any further insight.

@enomado
Copy link

enomado commented Apr 6, 2018

@tuukkamustonen oh I've just got your proposition. I'ts cludgy. I come to something like User.c.name (stealed from sqlalchemy). My temporary solution is to turn off typing in queries or cast User.name to any (sadly python have no typescript-style casts like (obj as any) for that purpose, but we have cast() function, heh)

@bySabi
Copy link

bySabi commented Jun 21, 2018

@tuukkamustonen, @enomado can you share a snippet of code of how you "by pass" mypy errors.
I'm stuck on this

model/__init__.py:5: error: Cannot find module named 'peewee'
model/__init__.py:17: error: Name 'SqliteDatabase' is not defined
model/__init__.py:20: error: Name 'Model' is not defined
model/__init__.py:26: error: Name 'IntegerField' is not defined
model/__init__.py:27: error: Name 'IntegerField' is not defined

@bySabi
Copy link

bySabi commented Jun 21, 2018

Well I could silence mypy creating a very simple local peewee.pyi , for my needs:
stubs/peewee.pyi

import typing

class Field:
    def __init__(self, **kwargs) -> None: ...

class CharField(Field): ...

class IntegerField(Field): ...

class DateField(Field): ...

class Model:
    def __init__(self, **kwargs) -> None: ...

class SqliteDatabase:
    def __init__(self, *args, **kwargs) -> None: ...

and tested with:

$ MYPYPATH=stubs/ mypy combs.py

At the end it seems with can add a simple peewee mypy silencer to https://github.com/python/typeshed

@bySabi
Copy link

bySabi commented Jun 21, 2018

Static type check is great tool and like unit test and code coverage you don't have to check it all.

Some kind of developers, like me, need all the help from tools that we can have.

To me type annotations is more useful than normal doc comments. It forces me to have them updated and follow type hints PEP 484 specs.

Python dynamism is great for create flexible libs, frameworks and tools like peewee. But in everyday jobs too much flexibility could be a problem, Type-Driven Development help me to be centered on the task.

@tuukkamustonen
Copy link
Author

tuukkamustonen commented Jun 25, 2018

@bySabi I think your problem is different than what we discussed here? You had:

model/__init__.py:5: error: Cannot find module named 'peewee'

That probably just tells that your (virtual)environment/PYTHONPATH is somehow messed up, because it's not finding peewee at all, thus giving errors also for stuff imported from it? You shouldn't be getting those errors in the first place.

I don't think this issue can be handled through type stubs (.pyis) as you still cannot provide different types for instance variable vs. class variable, something like ClassInstanceVar (see python/mypy#1097 for discussion, there's also a hacky solution by @enomado).

@bySabi
Copy link

bySabi commented Jun 26, 2018

@tuukkamustonen it's possible that I missed something on mypy settings but virtual env is working fine when the project is runned.

I think the problem with objects: SqliteDatabase, Model, IntergerField ... it's because they were imported using from peewee import * and that's why I could not apply ignore_missing_imports mypy setting on peewee. Lesson learned! : - )

In the end I could solve it using above peewee.pyi stub file.

And your are right, my typing hint problem don't have nothing to do with this issue. But anyway annotate models with types IMO is lot of duplicate work cause peewee type Fields (IntergerField, CharField, ...) will do the typecheck/validation at runtime.

@ce603
Copy link

ce603 commented May 15, 2020

I would also like to request this, because it helps with code completion.

For example, consider the code:

my_user = User.select().first()

This returns a <Model: User>, but it would be great if it type hinted to a User object, because on the next line of code, I would get a list of functions available to user (for example, if the User class had a function user_function(), I would see user_function() in the list of suggestions in the code completion hints. I know that I can specify this using # type: User, but I'd prefer it be automatic.

Also, is it possible to specify a return collection type? For example, in SQLAlchemy, I can do something like

orders = relationship("Order", back_populates="user", collection_class=Orders)  # type: Orders

and the orders will be returned as an Orders object, which is a custom class object that inherits from InstrumentedList. This allows me to add special functions, such as .most_recent(), .to_df(), etc...

Repository owner deleted a comment from mattrobin Jun 13, 2021
Repository owner locked and limited conversation to collaborators Jun 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants