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

Added custom priorities and Cut() validatable to stop validation immediately. #39

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

CptBinho
Copy link

@CptBinho CptBinho commented Apr 3, 2014

Hi! Here's the summary of my fork, which I think could be useful for inclusion in schema.

Validatables can have custom priorities so that we can control exactly in what order keys are tested in a dictionary schema.

The Cut() class can be used to fail validation immediately when we don't want a certain key to be caught by a more general rule. An example is given in the docstring for the Cut() class, and a test case is included.

Also, all validatable classes now share a common base class (SchemaBase), and instead of checking for the presence of a "validate" attribute in an object (which may be present but not necessarily intended for validation with schema) we check if the object is an instance of SchemaBase. The user can still provide custom validatables by merely adding SchemaBase to their bases.

Anyway, the cut was the main reason why I did this fork, as it was useful to me in a CLI I was doing with the amazing docopt :)

Cheers,
Rui

@keleshev
Copy link
Owner

keleshev commented Apr 3, 2014

There's quite a lot of changes. I have not yet formed opinions on all of them, but one thing I'm not quite happy is the base class.

The problem with the base class is that it forces people to use multiple inheritance, in order to make their things "validatable":

class Player(db.Model, SchemaBase):

    def validate(self, data):
        ...

So this is one disadvantage of a base class, which is minor, but I don't see any advantages to it.

In other languages I would definitely make an interface Validatable or something like that, but not a base class. Since we are using Python I would rather rely on duck-typing. What do you think?

@CptBinho
Copy link
Author

CptBinho commented Apr 3, 2014

I understand what you're saying, and I agree with it, but the reason why I added the base class is because the "validate" attribute may be part of the object for some other reason that has nothing to do with schema, and so the library would probably give the user a strange error message. In the worst case, validation would succeed with data that should not be valid (?!? not sure about this).

I know that people frown upon multiple inheritance, but in this case it would be totally free in the sense that all you need to do is simply add SchemaBase (I too prefer the name Validatable) to your class' bases and magic would ensue.

Another thing I noticed now is that the 'priority' attribute that is in the base class would be lost as well. 'priority' is used by the priority() function, which allows us to specify custom priorities for validatables, but they have a fallback value of 4 in SchemaBase.

So with these arguments, I leave it up to you to decide which alternative is better, as I believe both have advantages and disadvantages.

@keleshev
Copy link
Owner

keleshev commented Apr 3, 2014

Yeah, I need to think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants