Skip to content

Conv accept #115

Merged
merged 4 commits into from Mar 29, 2013

3 participants

@Harut
Harut commented Mar 27, 2013
  • Removed hack with wrapping to_python method
  • Explicit call of to_python when error handling and required check is not needed
  • No more mess with accept and to_python methods, Converter.convert method is added: to_python just converts value and may raise ValidationError, convert catches validation errors, checks for required and applies filters

minuses:

  • we have to check everywhere in the code, whether to use convert or accept methods
@ods

accept() method seems OK, but convert() is certainly a bad name.

Any ideas for the name?

@ods

Why you didn't make this in main branch? It's not related to the rest changes.

@Harut

Essentially, the content of try: block is a part of to_python flow, not accept. If we will move it there, the code becomes more clear. But I don't know how to implement it without any wrappings or other ugly staff.

@riffm
riffm commented Mar 28, 2013

👍

@ods
ods commented Mar 29, 2013

Good:

  • we now can fix long standing problems
  • simple and obvious interface of to_python for most cases and more flexible interface of accept for rare occasions when we need to fill errors for several fields

Bad:

  • no symetry: we have 2 interface methods for one direction and 1 for opposite.
@Harut
Harut commented Mar 29, 2013

simple and obvious interface of to_python

The thing still unsettling me about these changes is that to make the interface really obvious and consistent, as I wrote before, we have to force the content of try: block to be a part of to_python. I.e. required check and call of filters and validators.

How to implement it?

  • Wrap to_python as it was before? Or do some stuff with metaclasses as equal? Looks like hack
  • Implement this checks in Converter.to_python and force call Converter.to_python(self, value) every time the method is redefined? It's annoying and fragile.

May be anyone knows straight way?

@Harut
Harut commented Mar 29, 2013

I mean, if we write for exmaple:

    conv = Char(striptags, limit(3,250))
    # we expect that:
    conv.to_python('<p>Text</p>') == 'Text'
    conv.to_python('a') # raises ValidationError by length

In current (in the branch) implementation we will get

    conv.to_python('<p>Text</p>') == '<p>Text</p>'
    conv.to_python('a') == 'a'

    conv.accept('<p>Text</p>') == 'Text'
    conv.accept('a') == None # form.errors = {'name': 'blabla the string is too short'}
@Harut Harut merged commit 73f4a9b into master Mar 29, 2013
@Harut Harut deleted the conv-accept branch Mar 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.