views:

53

answers:

3

Python noob here,

Currently I'm working with SQLAlchemy, and I have this:

from __init__ import Base
from sqlalchemy.schema import Column, ForeignKey
from sqlalchemy.types import Integer, String
from sqlalchemy.orm import relationship

class User(Base):
    __tablename__ = "users"
    id = Column(Integer, primary_key=True)
    username = Column(String, unique=True)
    email = Column(String)
    password = Column(String)
    salt = Column(String)
    openids = relationship("OpenID", backref="users")

User.__table__.create(checkfirst=True)

#snip definition of OpenID class

def create(**kwargs):
    user = User()
    if "username" in kwargs.keys():
        user.username = kwargs['username']
    if "email" in kwargs.keys():
        user.username = kwargs['email']
    if "password" in kwargs.keys():
        user.password = kwargs['password']

    return user

This is in /db/users.py, so it would be used like:

from db import users
new_user = users.create(username="Carson", password="1234")
new_user.email = "[email protected]"
users.add(new_user) #this function obviously not defined yet

but the code in create() is a little stupid, and I'm wondering if there's a better way to do it that doesn't require an if ladder, and that will fail if any keys are added that aren't in the User object already. Like:

for attribute in kwargs.keys():
    if attribute in User:
        setattr(user, attribute, kwargs[attribute])
    else:
        raise Exception("blah")

that way I could put this in its own function (unless one hopefully already exists?) So I wouldn't have to do the if ladder again and again, and so I could change the table structure without modifying this code.

Any suggestions?

+2  A: 

My suggestion would be to not simplify it any further. You risk stepping on important object structures if you assign arbitrary attributes.

The one simplification I would do is to drop .keys() when you use it on a dict; both containment checking and iteration already use the keys.

...

On second thought, you could have a class attribute that contains known safe attributes, and then check this attribute within the function, and use setattr() on the instance.

Ignacio Vazquez-Abrams
yes, I'm looking for a way to check whether an attribute exists within a class before writing to it. Is there a builtin way to do this in Python? Or better, in SQLAlchemy (so to only allow valid columns to be set?) I suppose I could just create a dict, but I was trying to avoid duplicating the table structure
Carson Myers
You can use `hasattr()` to see if an object has a given attribute, but that won't prevent them from stomping on the object's bits.
Ignacio Vazquez-Abrams
What if I check the type of the attribute? At class definition, the column attributes are being set to Column objects -- but when I assign to them, I'm just assigning strings... Is there magic happening? Would checking that the attribute is a Column work?
Carson Myers
Unfortunately `Column` is most likely a descriptor, which is basically a black box for the type that it returns. I'd look for something else on the object that lists what the valid field names are; Django's ORM has this, so I'd be surprised if SQLAlchemy didn't.
Ignacio Vazquez-Abrams
What does Django call it? I just am not sure where to start looking, I've checked the output of `dir()` on a bunch of stuff but it seems to not really lead anywhere
Carson Myers
Django calls it `._meta.fields`, but Django's ways are... special.
Ignacio Vazquez-Abrams
haha, fair enough. Thanks for the help
Carson Myers
+1  A: 

If you don't need to cover inherited attributes,

def create(**kwargs):
    keys_ok = set(User.__dict__)
    user = User()
    for k in kwargs:
        if k in keys_ok:
            setattr(user, k, kwargs[k])

If you do need to cover inherited attributes, inspect.getmembers can help (with a custom predicate to avoid members whose names start with underscore, or others you want to ensure can't be set this way).

I would also consider (at least) giving a warning if set(kwargs) - set(keys_ok) is not empty -- i.e., if some of the named arguments passed to create cannot be set as arguments in the created instance; that can't be a good thing...!-)

Alex Martelli
is a predicate a function? Like a filter?
Carson Myers
@Carson, it's a function that returns a true value for members you want, a false value for members you don't want. "a filter" is a _way_ overloaded term (e.g, built-in `filter` **applies** a predicate to select some items of a sequence -- so it _applies_ a predicate, obviously it's not **like** a predicate...!-) so your second question becomes just too ambiguous to answer precisely.
Alex Martelli
I see. Well, I wound up writing a list comprehension like so: `[i for i in User.__dict__ if not i.startswith("_")]` -- is this very pythonic? Would using getmembers and a lambda be more pythonic?
Carson Myers
@Carson, looking in the `__dict__` is fine **if** you don't need to support inheritance -- it doesn't even _look_ at members inherited from the class's bases and other ancestors. `inspect.getmembers` walks the whole inheritance tree to support inheritance. If you don't care about inheritance, using the `__dict__`'s OK.
Alex Martelli
+2  A: 

Actually the declarative base class already inserts the exact constructor that you are looking for, as documented in the declarative modules docs. So just doing User(username="Carson", password="1234") will do what you want and User(something_not_an_attribute='foo') will raise an exception.

Ants Aasma