Added decorators for checking values of input to property setters (Issue-38) #46
Conversation
| @@ -1,5 +1,6 @@ | |||
| import xml.etree.ElementTree as ET | |||
| from .exceptions import UnpopulatedPropertyError | |||
| from .property_decorators import property_not_nullable | |||
There was a problem hiding this comment.
At first glance the code feels ok, but I think there might be a better name than "property_decorator" which both have meanings of their own.
I'll do a more detailed review in a bit
|
I prefer the decorator having a name that indicates it's intent and not the fact that it happens to be testing properties or is a decorator :) I'm not a stickler but it'll make it easier to understand down the road. @validate_schema or @validate_model or something like that feels much closer to what it does. The second question is can we do a single decorator that takes in parameters on what it validates (Django Forms, I believe, is similar to this) @validate_model(enum=Auth, is_null=False) It makes the decorator explosion a little more manageable. |
|
I'd much rather the individual decorators than one über one. If we have a single one, then we have to account for all possible combinations. That's error prone. If we have multiple decorators, we can compose them into whatever combination we need. |
|
I also prefer the individual decorators. I tried writing one but as Russell pointed out it ended up being an explosion and not nearly as easy to read. |
|
Fair point on wanting them to be composable -- I think that can be done without having an uber method though. Django does it by having one decorator that takes a list of functions and that does the validation for the model by applying those functions. So for us it could be: Each function is the composable set of checks we've got today, and if we add more in the future it's a little easier to control the order in which the validations are applied -- and not make functions look giant. |
|
Order is not so important except to make sure the setter is first (still an issue in yours). |
…ableau#45) Fixes tableau#42 tableau#46 * Initial attempt at enabling reading the columns from the datasource * Fixing pep8 errors for EOFEOL * Changing to OrderedDict for getting columns * Add documentation for the various column attributes * rename column to field * Fixed tableau#46 encode apostrophes in field names * Enable multilook up for Fields * Rename properties on the field based on feedback given in tableau#45
No description provided.