Proposed addition
Paul Johnston
I have some code I keep using that I hope you'll add to Elixir.
Essentially it is a "deep_set" method. So while obj.set() sets
properties just on that object, deep_set takes a hash/list hierarchy
and updates relations as well.
I just proposed adding this to core SA and Michael declined. Still
think this is a useful feature for an ORM so hope you will include it.
If you decide this is worth pursuing, I will do the work to produce a
patch with unit tests.
Paul
Jonathan LaCour
I think it might be valuable, but I think in order for it to
be accepted, I'd prefer it to be bi-directional. From what I
understand, this code just allows you to perform inserts and updates
based upon what appears to be a JSON data structure. Ideally, I'd
like it to be able to generate that structure as well.
If it did this, I'd be highly likely to accept such an extension.
It'd be nice if it was a statement that attached this behavior to
the decorated entity.
--
Jonathan LaCour
http://cleverdevil.org
Paul Johnston
> I think it might be valuable, but I think in order for it to
> be accepted, I'd prefer it to be bi-directional.
Single parameter, "deep", which defaults to an empty dict, meaning
just the object is JSONified. If you specify keys in the dict, those
relations are included in the JSON output. And if you specify values,
those values are passed as the "deep" parameter when to_json is called
on the related objects.
Contact.get(3).to_json(deep={'phone':{}})
I'll have a go at producing a patch during the week.
Paul
Paul Johnston
> > I think it might be valuable, but I think in order for it to
> > be accepted, I'd prefer it to be bi-directional.
functionality to existing Entity.set. I've done unit tests and all.
I was going to create a ticket for this, but I'm having trouble -
guest doesn't let me create tickets and I can't see a register link.
Will have a look at to_json now, although that's less important to me.
Paul
Paul Johnston
> Ok, I've just implemented the deep_set approach, adding the
> functionality to existing Entity.set. I've done unit tests and all.
Just the changes to Entity.set:
http://pajhome.org.uk/deep_set.patch
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.
Regards,
Paul
Gaetan de Menten
> > 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
Paul Johnston
> Other, minor complaints include (in arbitrary order and without any
> "gentle wrapping" -- I hope you don't take them badly:
comments are fair points :-)
> could see your code only handle columns and *list*
> properties/relationships, and thus cannot be used to set a ManyToOne
> relationship.
> - Is the "dbdata.remove(delobj)" line useful? (it seem useless to me
> but I could be wrong)
make them pass. I reckon leave it in.
> Also, I'd personally prefer if the deep_set was a method on the entity
classes as it is, which may be important in the future when you can
have relationships between Elixir and raw-SA classes. On balance,
think I'll go with your suggestion; we can always backtrack in the
future.
> (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
so simple sets remain performant?
> Seem pretty clean to me. What don't you like about it?
in.
Stay tuned for the next patch!
Paul
Gaetan de Menten
> > (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
>
> Yes, those are better names. In fact, shall we leave set() as it is,
> so simple sets remain performant?
Yes, that's what I had in mind.
> > Seem pretty clean to me. What don't you like about it?
>
> The deep parameter seems a bit weird. Anyway, screw it, I'll leave it
> in.
In fact, me too. But I thought it would be easily fixable by just
renaming it to something else (keys?) and using it without spelling
out the kwarg.
Paul Johnston
Ok, I've done a lot of what you suggested and put it at:
http://elixir.ematia.de/trac/ticket/40
Still using getattr(mapper, '_Mapper__props'), I can't see any other
way to get the names of relations.
Not done a back-and-forth test, as it doesn't yet run clear back-and-
forth - the id columns get set by the database, and it's not possible
to pre-set them with the current interface.
Not made it py2.3 compatible as yet - downloading it as I type.
I guess the other thing that's needed is updates to the documentation.
Anyway, I've done a lot of work to tidy this up already, so I hope you
can apply it soon. Happy to done some further tidy-ups once we've got
an initial version.
Paul
On Apr 22, 8:39 pm, "Gaetan de Menten" <gdemen...@gmail.com> wrote:
Paul Johnston
> Not made it py2.3 compatible as yet - downloading it as I type.
Paul
Gaetan de Menten
> Ok, I've done a lot of what you suggested and put it at:
> http://elixir.ematia.de/trac/ticket/40
>
> Still using getattr(mapper, '_Mapper__props'), I can't see any other
> way to get the names of relations.
I've fixed that. No worries...
> Not done a back-and-forth test, as it doesn't yet run clear back-and-
> forth - the id columns get set by the database, and it's not possible
> to pre-set them with the current interface.
I'm not sure it make sense, but we could drop those before creating
the instances, but still remember those values to build a lookup
mapping, so that further data referencing the same ids reusing the
same object instances (whatever the new ids will be).
> I guess the other thing that's needed is updates to the documentation.
> Anyway, I've done a lot of work to tidy this up already, so I hope you
> can apply it soon. Happy to done some further tidy-ups once we've got
> an initial version.
Applied now, thanks! Btw: next time, try to not revert the last commit
in your patch ;-).
Paul Johnston
> Applied now, thanks! Btw: next time, try to not revert the last commit
> in your patch ;-).
iterate_properties.
> I'm not sure it make sense, but we could drop those before creating
> the instances, but still remember those values to build a lookup
Couple of questions to close off:
Do I need to do anything for the docs? I guess the doc strings will be
picked up automatically.
Any idea when you're likely to do the next release?
Thanks for all your help getting to this point. Hope we get to work
together on something else,
Paul
Gaetan de Menten
> Couple of questions to close off:
> Do I need to do anything for the docs?
No, I don't think anything more than the docstring is needed for this.
> I guess the doc strings will be picked up automatically.
Indeed.
> Any idea when you're likely to do the next release?
I've a couple new features I'd like to implement before releasing
(concrete inheritance among others). So that'll be either tomorrow if
I can finish what I have started or next week (I'll be kinda busy
during the rest of the week).
> Thanks for all your help getting to this point. Hope we get to work
> together on something else,
I'd be glad to.