On Tue, Apr 22, 2008 at 9:39 AM, Paul Johnston <[EMAIL PROTECTED]> wrote:
> > Ok, I've just implemented the deep_set approach, adding the > > functionality to existing Entity.set. I've done unit tests and all. > > I've put the patches on my website. > > Just the changes to Entity.set: > http://pajhome.org.uk/deep_set.patch That's a pretty interesting patch. Thanks a lot already. I have a few complaints however. The most important one being that, as far as I could see your code only handle columns and *list* properties/relationships, and thus cannot be used to set a ManyToOne relationship. Other, minor complaints include (in arbitrary order and without any "gentle wrapping" -- I hope you don't take them badly: - You use "getattr(mapper, '_Mapper__props')", where I think mapper.iterate_properties might be more appropriate. - The following test: "data[rname] is not None" shouldn't be there IMO (so that you can set a relationship to "NULL"). - Is the "dbdata.remove(delobj)" line useful? (it seem useless to me but I could be wrong) - In the setup for the tests, you use a backref instead of the more Elixir-ish (and IMHO readable) OneToMany. - There is no test using both "set" and "to_json". I'd really appreciate a back-and-forth test. - The code is not python2.3 compatible (apparently there are some users still interested in that). Also, I'd personally prefer if the deep_set was a method on the entity (probably named "from_dict") and the "to_json" method was renamed to "to_dict" as the output is not really json and I like to have corresponding/consistent names for opposite methods. I think this patch will be a great addition, once those little details are ironed out. > This patch also includes the to_json method: > http://pajhome.org.uk/deep_set_json.patch > > Thinking about it, the to_json stuff doesn't seem as clean as the .set > changes. I'd advocate only applying deep_set.patch. Seem pretty clean to me. What don't you like about it? -- Gaëtan de Menten http://openhex.org --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "SQLElixir" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/sqlelixir?hl=en -~----------~----~----~----~------~----~------~--~---
