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

Empty value handling in colander #35

Closed
wants to merge 1 commit into from
Closed

Empty value handling in colander #35

wants to merge 1 commit into from

Conversation

adroullier
Copy link

  1. For a better compatibility with backend modules or data storage systems it was much more convenient to allow "None" and empty strings for Tuple, Number, Date, DateTime, Time nodes.
    The current version raises an Invalid exception on serialization.

  2. Parse Date, DateTime and Time strings in serialize(). Again for a better compatibility.
    For example SQLite reads dates/datetimes as strings by default.

Maybe this is not really required for standalone colander usage. But it definetly helps to make deform integration easier.
Though, I don't think this will break anything. Maybe you have a look.

Arndt.

…ize(). Handle Date, DateTime, Time as strings in serialize().
@dnouri
Copy link
Member

dnouri commented Mar 28, 2012

Sorry, realised the overlap between this and #45 only now. #45 fixes the None case for Date, DateTime, Time and String. You're saying it should include Tuple, which makes sense.

I'm not sure about 2), which seems like it's happening in the wrong layer here. Maybe deriving from the colander types, or having your models do the parsing is better.

@adroullier
Copy link
Author

  1. Yes, that's right. You can expect models to support DateTime types. And even if they don't it should be quite easy to change it since models have type information on their own.

I was referring to data sources in general and to use colander to process data from any source and store in the model.
For example python csv, xml, json or any other module. None of these support DateTime but use simple string representations. (The DateTime changes only apply to incoming data, the processing remains the same)

Right now colander is too restrictive to be used this way without subclassing or additional layers. Well, in this context I think these additions make sense.

@mcdonc
Copy link
Member

mcdonc commented Apr 27, 2012

I had a look at this before releasing 0.9.8, and I think most of it's sensible but I didn't merge the tuple and datetime-as-string changes because:

  1. Probably need to get rid of except: pass in datetime serializations in favor of catching a real exception.
  2. Does it really make sense to consider the empty string a valid serializable object for a tuple? Maybe if it just said "if not appstruct: return null" I'd probably (illogically) feel better about it. ;-) It probably doesn't matter much. Changes to serialization can be pretty loose because no validation happens anyway.

Would you mind working up another patch that applies to the master and gets rid of the overgeneral exception catching?

@kiorky
Copy link
Member

kiorky commented Sep 21, 2012

@mcdonc close or not ?

@mcdonc
Copy link
Member

mcdonc commented Sep 21, 2012

No harm in leaving it open for now, we still haven't addressed any of the issues it fixes.

@mcdonc
Copy link
Member

mcdonc commented Aug 10, 2013

I'm going to close this, as I just fixed another bug that had to do with overbroad usage of "None" as a sentinel somewhere. For better or worse, colander.null is the only valid sentinel for now.

@mcdonc mcdonc closed this Aug 10, 2013
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.

None yet

4 participants