address @wyattanderson comments

Author: jnak

graphene_sqlalchemy/converter.py

@@ -102,14 +102,9 @@ def inner(fn):
 
 def convert_sqlalchemy_column(column_prop, registry, **field_kwargs):
     column = column_prop.columns[0]
-    if 'type' not in field_kwargs:
-        field_kwargs['type'] = convert_sqlalchemy_type(getattr(column, "type", None), column, registry)
-
-    if 'required' not in field_kwargs:
-        field_kwargs['required'] = not is_column_nullable(column)
-
-    if 'description' not in field_kwargs:
-        field_kwargs['description'] = get_column_doc(column)
+    field_kwargs.setdefault('type', convert_sqlalchemy_type(getattr(column, "type", None), column, registry))
+    field_kwargs.setdefault('required', not is_column_nullable(column))
+    field_kwargs.setdefault('description', get_column_doc(column))
 
     return Field(
         resolver=_get_attr_resolver(column_prop.key),

graphene_sqlalchemy/tests/test_types.py

@@ -114,14 +114,17 @@ def test_sqlalchemy_override_fields():
     def convert_composite_class(composite, registry):
         return String()
 
-    class ReporterType(SQLAlchemyObjectType):
+    class ReporterMixin(object):
+        # columns
+        first_name = ORMField(required=True)
+        last_name = ORMField(description='Overridden')
+
+    class ReporterType(SQLAlchemyObjectType, ReporterMixin):
         class Meta:
             model = Reporter
             interfaces = (Node,)
 
         # columns
-        first_name = ORMField(required=True)
-        last_name = ORMField(description='Overridden')
         email = ORMField(deprecation_reason='Overridden')
         email_v2 = ORMField(prop_name='email', type=Int)
 
@@ -151,9 +154,10 @@ class Meta:
             use_connection = False
 
     assert list(ReporterType._meta.fields.keys()) == [
-        # First the ORMField in the order they were defined
+        # Fields from ReporterMixin
         "first_name",
         "last_name",
+        # Fields from ReporterType
         "email",
         "email_v2",
         "column_prop",

graphene_sqlalchemy/types.py

@@ -27,22 +27,59 @@
 class ORMField(OrderedType):
     def __init__(
         self,
-        type=None,
         prop_name=None,
+        type=None,
+        required=None,
         description=None,
         deprecation_reason=None,
-        required=None,
         _creation_counter=None,
         **field_kwargs
     ):
+        """
+        Use this to override fields automatically generated by SQLAlchemyObjectType.
+        Unless specified, options will default to SQLAlchemyObjectType usual behavior
+        for the given SQLAlchemy model property.
+
+        Usage:
+            class MyModel(Base):
+                id = Column(Integer(), primary_key=True)
+                name = Column(String)
+
+            class MyType(SQLAlchemyObjectType):
+                class Meta:
+                    model = MyModel
+
+                id = ORMField(type=graphene.Int)
+                name = ORMField(required=True)
+
+        -> MyType.id will be of type Int (vs ID).
+        -> MyType.name will be of type NonNull(String) (vs String).
+
+        Parameters
+        - prop_name : str, optional
+            Name of the SQLAlchemy property used to resolve this field.
+            Default to the name of the attribute referencing the ORMField.
+        - type: optional
+            Default to the type mapping in converter.py.
+        - description: str, optional
+            Default to the `doc` attribute of the SQLAlchemy column property.
+        - required: bool, optional
+            Default to the opposite of the `nullable` attribute of the SQLAlchemy column property.
+        - description: str, optional
+            Same behavior as in graphene.Field. Defaults to None.
+        - deprecation_reason: str, optional
+            Same behavior as in graphene.Field. Defaults to None.
+        - _creation_counter: int, optional
+            Same behavior as in graphene.Field.
+        """
         super(ORMField, self).__init__(_creation_counter=_creation_counter)
         # The is only useful for documentation and auto-completion
         common_kwargs = {
-           'type': type,
-           'prop_name': prop_name,
-           'description': description,
-           'deprecation_reason': deprecation_reason,
-           'required': required,
+            'prop_name': prop_name,
+            'type': type,
+            'required': required,
+            'description': description,
+            'deprecation_reason': deprecation_reason,
         }
         common_kwargs = {kwarg: value for kwarg, value in common_kwargs.items() if value is not None}
         self.kwargs = field_kwargs
@@ -52,7 +89,27 @@ def __init__(
 def construct_fields(
     obj_type, model, registry, only_fields, exclude_fields, connection_field_factory
 ):
+    """
+    Construct all the fields for a SQLAlchemyObjectType.
+    The main steps are:
+        - Gather all the relevant attributes from the SQLAlchemy model
+        - Gather all the ORM fields defined on the type
+        - Merge in overrides and build up all the fields
+
+    Parameters
+    - obj_type : SQLAlchemyObjectType
+    - model : the SQLAlchemy model
+    - registry : Registry
+    - only_fields : tuple[string]
+    - exclude_fields : tuple[string]
+    - connection_field_factory : function
+
+    Returns
+    - fields
+        An OrderedDict of field names to graphene.Field
+    """
     inspected_model = sqlalchemyinspect(model)
+    # Gather all the relevant attributes from the SQLAlchemy model
     all_model_props = OrderedDict(
         inspected_model.column_attrs.items() +
         inspected_model.composites.items() +
@@ -61,31 +118,37 @@ def construct_fields(
         inspected_model.relationships.items()
     )
 
+    # Filter out excluded fields
     auto_orm_field_names = []
     for prop_name, prop in all_model_props.items():
         if (only_fields and prop_name not in only_fields) or (prop_name in exclude_fields):
             continue
         auto_orm_field_names.append(prop_name)
 
-    # TODO Get ORMField fields defined on parent classes
-    custom_orm_fields_items = []
-    for attname, value in list(obj_type.__dict__.items()):
-        if isinstance(value, ORMField):
-            custom_orm_fields_items.append((attname, value))
+    # Gather all the ORM fields defined on the type
+    custom_orm_fields_items = [
+        (attname, value)
+        for base in reversed(obj_type.__mro__)
+        for attname, value in base.__dict__.items()
+        if isinstance(value, ORMField)
+    ]
     custom_orm_fields_items = sorted(custom_orm_fields_items, key=lambda item: item[1])
 
+    # Set the prop_name if not set
     for orm_field_name, orm_field in custom_orm_fields_items:
         prop_name = orm_field.kwargs.get('prop_name', orm_field_name)
         if prop_name not in all_model_props:
             raise Exception('Cannot map ORMField "{}" to SQLAlchemy model property'.format(orm_field_name))
         orm_field.kwargs['prop_name'] = prop_name
 
+    # Merge automatic fields with custom ORM fields
     orm_fields = OrderedDict(custom_orm_fields_items)
     for orm_field_name in auto_orm_field_names:
         if orm_field_name in orm_fields:
             continue
         orm_fields[orm_field_name] = ORMField(prop_name=orm_field_name)
 
+    # Build all the field dictionary
     fields = OrderedDict()
     for orm_field_name, orm_field in orm_fields.items():
         prop_name = orm_field.kwargs.pop('prop_name')