typ or schema_type

[tisdall]

There appears to be two "type" properties on schemas. It appears that schema_type was intended for use with inheritance and typ is the keyword argument you pass to the init. However, having two properties that do the same sort of purpose leads to some messy stuff.

example:

>>> import colander
>>> x = colander.SchemaNode(typ=colander.List, schema_type=colander.Int)
>>> x.typ
<class 'colander.List'>
>>> x.schema_type
<class 'colander.Integer'>
>>> 

You can also create a subclass that defines the schema_type and then just call the init with typ causing the split again.

related issue: #150

Is there any reason we can't have one be an alias of the other and just have a single value?

[mmerickel]

I think you should also consider my comments in #168 with regards to where to go from here.

[tisdall]

one minor correction: schema_type is expected to be a callable with no arguments and typ is expected to just be a property. So aliasing would be something like schema_type = lambda: typ.

@mmerickel - I'll take a look... I'm not really familiar with the unknown functionality.

[tisdall]

Finally the pattern of doing self.dict.update(kw) in SchemaNode.init is pretty terrible as it turns all of these very real errors into silent no-ops. This pattern should be reconsidered when it comes to a validation library.

I totally agree with this statement... But then a change would break backwards compatibility... I guess it could be done but we'd have to call it colander 2.0. Even the colander docs seem to be abusing this functionality: #231

I think the best that can be done without breaking b.c. in this case is better documentation and perhaps better naming. The fact that an instance has a schema_type() and a typ is really confusing. Maybe schema_type() should be renamed to default_schema_type_factory() (with the old one throwing a warning and pointing to the new) and add some actual doc strings explaining it's purpose. And then add a default_schema_type_kwargs as a dict that's passed to the factory in the init. Would you accept a PR to this effect?

[tisdall]

... only issue with the above suggestion is it destroys the symmetry that someone was going for here: http://docs.pylonsproject.org/projects/colander/en/latest/basics.html?highlight=schema_type#subclassing-schemanode

However, that example is already broken as passing in schema_type as an argument simply doesn't work right now.

[tisdall]

@mmerickel - Do you have any suggestions for dealing with the above doc example? Should we perhaps allow schema_type in the constructor as an alias for typ?

[mmerickel]

I think colander needs to do a better job of not allowing arbitrary kwargs in __init__. You shouldn't be allowed to pass schema_type, just like it should accept certain other options that are supposed to be passed to the typ instance itself. If this was validating properly I think you'd be complaining less about the differing names because I think they're mostly fine in their individual usage patterns.