Suggestion for new behavior of elixir.collection.EntityCollection.resolve().
Johannes Janssen
The method resolve() of elixir.collection.EntityCollection is used by
relations like ManyToOne(target, [...]). The relations use it to get a
corresponding entity defined by the string target. Atm. you either have
to use the full module path to address your target or if your are lucky
you can address it just by the classname under certain conditions.
This behavior is okay for those who just want to define all their
entities in one module. For clarity I instead prefer to split up my
model into submodules. Therefore I've to use full module path which is
very verbose and leads to problems, if you want to use your model module
or package in a different application.
I propose a patch for resolve(), which makes to major changes:
User defined resolve function:
I think you should be able to pass a function through the options which
replaces the default resolve function. This is similar to the tablename
option, which also accepts a function. With this patch you are able to
pass a function with the option "resolve_function".
The function will be passed same information as the original resolve
method and is expected to return an Entity object. But an fallback to
default resolve behavior is possible by returning None. If you want
instead to have an exception raised, because you could not resolve the
target with your own function, you've got to raise the exception in the
user defined function.
Resolving the module path:
My second idea is to change the way module paths are resolved by
default. Basically I would like to have tow ways to do that: relative
and (pseudo) absolute paths. An relative path starts with a dot and is
relative to the entity which is passed to resolve(key, entity) or better
to entity.__module__. Therefore it is only possible to use relative
paths when an entity is passed to resolve().
The absolute path instead must not start with a dot. They are by the
default real absolute path (that means they are relative to __main__).
Using "resolve_root" option you can change this. Think of chroot in
POSIX. You can define your pseudo root module. So that all path a
relative to that one.
(What I said about module paths (not) starting with a dot is not quite
true. "..foo.bar" will still be an abs path and while "...foo.bar" is an
rel path. Got it? Have a look at the example!)
Okay, allot information about how my patch works here. So what are they
advantages?
- This way paths are unique (it is clear whether a rel or abs path is
meant).
- Think of a db package where define resolve_root as __name__ through
options_defaults: it doesn't matter anymore whether your db packages
lives in "my_app.db" or in "meta_app.my_app.db". Atm. you would have to
change all the btw. very verbose absolute paths if you are forced to use
absolute paths.
Thanks for reading this far ;) What do you think about it? Please have a
look at the patch, which is against svn trunk r455. Has this patch any
chance to get into elixir? Any further suggestions concerning the
implementations. For backwards compatibility you might give users the
possibility to switch back manually to old behavior and give them a
deprecation warning (for a few versions if there are to be more;)).
Also have a look at the example I provide. It shows how the new path
resolving works and what advantages it might offer in a multi-file model
definition.
Kind regards
Johannes
Jonathan LaCour
> I propose a patch for resolve(), which makes to major changes:
>
> User defined resolve function
I like this particular aspect of the patch, in theory. It allows for the
user to define the entity resolution process to their heart's content.
> Resolving the module path
I don't particularly like this idea. I think that the current approach
is simpler and more easy to understand. If we accept the patch for the
user-defined resolve function, you could (in theory) implement this
behavior in your own project if you wanted to.
> For backwards compatibility you might give users the
> possibility to switch back manually to old behavior and give them a
> deprecation warning (for a few versions if there are to be more;)).
This is another reason I dislike the second part of your patch.
Gaetan, what are your thoughts? Anyone else have any opinions on the
subject?
--
Jonathan LaCour
http://cleverdevil.org
Johannes Janssen
Jonathan LaCour schrieb:
> Johannes Janssen wrote:
>
>
>> I propose a patch for resolve(), which makes to major changes:
>>
>> User defined resolve function
>>
>
> I like this particular aspect of the patch, in theory. It allows for the
> user to define the entity resolution process to their heart's content.
>
Btw. a user-defined resolve function should also get passed the
EntityCollection, this not implemented in my patch.
>> Resolving the module path
>>
>
> I don't particularly like this idea. I think that the current approach
> is simpler and more easy to understand. If we accept the patch for the
> user-defined resolve function, you could (in theory) implement this
> behavior in your own project if you wanted to.
>
Yeah I could live with that, but I don't think that the current solution
is the best approach. That's of course just my own opinion and I don't
what to force anyone to use my approach.
We may discuss what is the best way to resolve path and what notation
you want to use. My way of doing that is just one of many possible ones.
I assume elixir's users want to split up their db definitions into
submodules and elixir should support them to do this. Although it
possible through full paths there are still the mentioned disadvantages.
My patch is somewhat inspired by
http://www.python.org/dev/peps/pep-0328/#guido-s-decision and therefore
it's imho quite intuitive.
>> For backwards compatibility you might give users the
>> possibility to switch back manually to old behavior and give them a
>> deprecation warning (for a few versions if there are to be more;)).
>>
>
> This is another reason I dislike the second part of your patch.
Okay if you don't want to break backwards compatibility you can keep the
old resolve method and provide alternatives. But not being backwards
compatible should imho not be a reason to hold back improvements.
Kind regards
Johannes
Gaetan de Menten
[...]
> I propose a patch for resolve(), which makes to major changes:
>
> User defined resolve function:
[...]
This is a good idea to address a real need. The thing is that I am not
sure this is the best approach to solve the problem. What I've been
using for my last project using elixir (a few months ago) is a custom
EntityCollection class (inheriting from the default one) overriding
the resolve method. We could probably add a hook in the default one so
that it's easier to provide a custom method though I'm unsure whether
it's worth it. My main concern about your approach is that I fail to
see any usecase where setting the resolve method on a per-entity basis
instead of per-collection makes sense. I might be lacking imagination
though. Can you provide an example of such a situation?
> Resolving the module path:
>
> My second idea is to change the way module paths are resolved by
> default. Basically I would like to have tow ways to do that: relative
> and (pseudo) absolute paths. An relative path starts with a dot and is
> relative to the entity which is passed to resolve(key, entity) or better
> to entity.__module__. Therefore it is only possible to use relative
> paths when an entity is passed to resolve().
> The absolute path instead must not start with a dot. They are by the
> default real absolute path (that means they are relative to __main__).
> Using "resolve_root" option you can change this.
Good ideas. I didn't think of those. I was using an alternate resolve
method to emulate your "resolve_root" option but since it's probably a
common use case, it's a nice idea to have such an option.
> (What I said about module paths (not) starting with a dot is not quite
> true. "..foo.bar" will still be an abs path and while "...foo.bar" is an
> rel path. Got it? Have a look at the example!)
Hmmm, I lost you there. The relative path idea seems good. Using a
leading dot seems good but this ".. is a no-op" idea seems artificial
and very different to how python's relative imports work, hence very
confusing. Honestly, I don't understand what's the point.
> Thanks for reading this far ;) What do you think about it? Please have a
> look at the patch, which is against svn trunk r455. Has this patch any
> chance to get into elixir?
Sure. Well, the patch as it is now probably won't but some
modifications to address your issues will. They won't go into 0.7
which is already months (years?) late but will be in 0.8. I can't
promise when that will come out though, but it will.
> For backwards compatibility you might give users the
> possibility to switch back manually to old behavior and give them a
> deprecation warning (for a few versions if there are to be more;)).
I don't want to break backward compatibility for the entity resolving
code *again* (already broke it from 0.5 to 0.6) just yet. But
providing a built-in alternate method wouldn't hurt. And if people use
it a lot, we can still make it the default later on.
--
Gaëtan de Menten
http://openhex.org
Gaetan de Menten
LaCour<jonatha...@cleverdevil.org> wrote:
> Johannes Janssen wrote:
>> User defined resolve function
> I like this particular aspect of the patch, in theory. It allows for the
> user to define the entity resolution process to their heart's content.
Well, as stated (albeit not very clearly) in my other email, it was
already possible to do that since 0.6 by providing a custom entity
collection.
Johannes Janssen
> On Wed, Aug 12, 2009 at 00:52, Johannes Janssen<ma...@johannes-janssen.de> wrote:
>
> [...]
>
>
>> I propose a patch for resolve(), which makes to major changes:
>>
>> User defined resolve function:
>>
>
> [...]
>
> This is a good idea to address a real need. The thing is that I am not
> sure this is the best approach to solve the problem. What I've been
> using for my last project using elixir (a few months ago) is a custom
> EntityCollection class (inheriting from the default one) overriding
> the resolve method. We could probably add a hook in the default one so
> that it's easier to provide a custom method though I'm unsure whether
> it's worth it. My main concern about your approach is that I fail to
> see any usecase where setting the resolve method on a per-entity basis
> instead of per-collection makes sense. I might be lacking imagination
> though. Can you provide an example of such a situation?
>
I agree with you that there is no need for having a user-defined resolve
function on per-entity basis. I'm just not that familiar how elixir
works. As I know now that you can implement your own resolve function by
using your own EntityCollection class, I see no real need adding code on
user-defined functions. Though it should be documented in some way how
to do it.
>> Resolving the module path:
>>
>> My second idea is to change the way module paths are resolved by
>> default. Basically I would like to have tow ways to do that: relative
>> and (pseudo) absolute paths. An relative path starts with a dot and is
>> relative to the entity which is passed to resolve(key, entity) or better
>> to entity.__module__. Therefore it is only possible to use relative
>> paths when an entity is passed to resolve().
>> The absolute path instead must not start with a dot. They are by the
>> default real absolute path (that means they are relative to __main__).
>> Using "resolve_root" option you can change this.
>>
>
> Good ideas. I didn't think of those. I was using an alternate resolve
> method to emulate your "resolve_root" option but since it's probably a
> common use case, it's a nice idea to have such an option.
>
I assume that you agree that having "resolve_root" on a per-entity basis
makes sense. There may be better names like "module_root" for that option.
>> (What I said about module paths (not) starting with a dot is not quite
>> true. "..foo.bar" will still be an abs path and while "...foo.bar" is an
>> rel path. Got it? Have a look at the example!)
>>
>
> Hmmm, I lost you there. The relative path idea seems good. Using a
> leading dot seems good but this ".. is a no-op" idea seems artificial
> and very different to how python's relative imports work, hence very
> confusing. Honestly, I don't understand what's the point.
>
Yeah, I totally don't understand how I came up with this, now. Perhaps
it was bit too late, when I did this. It is not how relative imports
work in python, but like paths in common OSes. Okay just ignore all the
".." stuff. That leaves this:
* absolute paths (must not start with a dot)
o they are "relative" to the resolve_root option
o you can't address classes living not within the module given
by resolve_root or any of it's submodules
o resolve_root defaults to __main__ module, making the paths
real absolute paths like in the current resolve method
* relative paths (must start with at least one dot)
o they are relative to the entity class passed to the resolve
function
o each additional dot at the beginning of the path has the
meaning of going up one module/package level
[...]
I append a new version of the resolve function implementing this
behavior (http://paste.pocoo.org/show/134345/).
Kind regards
Johannes
Gaetan de Menten
> Gaetan de Menten schrieb:
>> On Wed, Aug 12, 2009 at 00:52, Johannes Janssen<ma...@johannes-janssen.de> wrote:
[...]
>>> They are by the
>>> default real absolute path (that means they are relative to __main__).
>>> Using "resolve_root" option you can change this.
>> Good ideas. I didn't think of those. I was using an alternate resolve
>> method to emulate your "resolve_root" option but since it's probably a
>> common use case, it's a nice idea to have such an option.
> I assume that you agree that having "resolve_root" on a per-entity basis
> makes sense.
Hmm no. Seems like I wasn't clear enough. I meant it as an option (ie
argument to the constructor) on the EntityCollection class.
What would a use case for what you suggest be? If you specify a
specific resolve_root for *one* particular entity, you might as well
use the absolute paths. That feature would only make sense if you have
*lots* of relations to the same target module within the same entity
*and* that target module is not the current module (or one close to it
in the module hierarchy). This seems like a very unlikely combination
and not worth a feature in the default distribution (you could still
implement it as a custom collection if you wish).
> There may be better names like "module_root" for that option.
Indeed.
This new version makes sense to me. I've created ticket #93 to keep track of it.
Gaetan de Menten
See my comment about it on the ticket though.
Johannes Janssen
>> I assume that you agree that having "resolve_root" on a per-entity basis
>> makes sense.
>>
>
> Hmm no. Seems like I wasn't clear enough. I meant it as an option (ie
> argument to the constructor) on the EntityCollection class.
>
> What would a use case for what you suggest be? If you specify a
> specific resolve_root for *one* particular entity, you might as well
> use the absolute paths. That feature would only make sense if you have
> *lots* of relations to the same target module within the same entity
> *and* that target module is not the current module (or one close to it
> in the module hierarchy). This seems like a very unlikely combination
> and not worth a feature in the default distribution (you could still
> implement it as a custom collection if you wish).
>
For example, I thought of merging different projects, which have
different module roots. So that you just set the default option
reslove_root different depending whether you are in the former project A
part or in that of project B.
Also think of the problem, that if the resolve_root is defined on
collection basis, you can never "break out" using absolute path.
But you're still right, that those are very special use cases. They can
also be solve using a user-defined resolve function. Another solution
might be that you allow to provide a function, which generates the
module_root (like the tablename option).
I append a new patch against r455, adding a "module_root" argument to
EntityCollection and making some improvements in the resolve implementation.
General question: where should we discuss this, here or on trac?
Kind regards
Johannes
Gaetan de Menten
> Gaetan de Menten schrieb:
>>> I assume that you agree that having "resolve_root" on a per-entity basis
>>> makes sense.
>> Hmm no. Seems like I wasn't clear enough. I meant it as an option (ie
>> argument to the constructor) on the EntityCollection class.
>>
>> What would a use case for what you suggest be? If you specify a
>> specific resolve_root for *one* particular entity, you might as well
>> use the absolute paths. That feature would only make sense if you have
>> *lots* of relations to the same target module within the same entity
>> *and* that target module is not the current module (or one close to it
>> in the module hierarchy). This seems like a very unlikely combination
>> and not worth a feature in the default distribution (you could still
>> implement it as a custom collection if you wish).
> For example, I thought of merging different projects, which have
> different module roots. So that you just set the default option
> reslove_root different depending whether you are in the former project A
> part or in that of project B.
Hmmm, that makes some sense, but for that use case, I would rather
either use two different collections (using the two different
module_root) or use two different base classes with the module_root
option set as an option_default on them. That might not be possible at
the moment (I'm not really sure), but I'd rather fix that than
implement something the wrong way. The reasoning is that module_root
really feel like it only makes sense for a group of entities and not
for a single entity, and specifying it over and over on each entity
seems like dumb to me.
> Also think of the problem, that if the resolve_root is defined on
> collection basis, you can never "break out" using absolute path.
I thought at first that it was a feature, rather than a problem... But
on second thoughts, it might be a problem indeed. Let us imagine we
are in the following setup: all your project entities are define in a
"db" module, but you also use TurboGears (or whatever external
library/framework which provides some elixir entities) and its
provided "User" entity. Just because of that one entity, you couldn't
use the module_root option which would otherwise make a lot of sense.
I still don't like the per-entity module_root option, because inside
the entity using it (to set the module_root to None), you'd have to
use absolute paths (or relative paths) for all the relationships, and
not just the one needing to "break out". As a solution to this, it
*might* be a good idea to add a mechanism to "force" absolute paths,
even when the module_root is not None, for example by using a leading
"/", as in "/turbogears.identity.User" (or whatever the real path is).
> But you're still right, that those are very special use cases. They can
> also be solve using a user-defined resolve function.
> Another solution
> might be that you allow to provide a function, which generates the
> module_root (like the tablename option).
That's getting crazy... If it is evaluated on a per-entity basis, it'd
make more sense to just provide the option. If not, it doesn't make
any sense.
> I append a new patch against r455, adding a "module_root" argument to
> EntityCollection and making some improvements in the resolve implementation.
> General question: where should we discuss this, here or on trac?
As a rule of thumb, the "do we implement feature X (with behavior Y
and Z)?" should be discussed on the list, while
implementation/technical details (how do we implement feature X) and
patches should be directed to Trac tickets.
Johannes Janssen
>> Also think of the problem, that if the resolve_root is defined on
>> collection basis, you can never "break out" using absolute path.
>>
>
> I thought at first that it was a feature, rather than a problem... But
> on second thoughts, it might be a problem indeed. Let us imagine we
> are in the following setup: all your project entities are define in a
> "db" module, but you also use TurboGears (or whatever external
> library/framework which provides some elixir entities) and its
> provided "User" entity. Just because of that one entity, you couldn't
> use the module_root option which would otherwise make a lot of sense.
>
> I still don't like the per-entity module_root option, because inside
> the entity using it (to set the module_root to None), you'd have to
> use absolute paths (or relative paths) for all the relationships, and
> not just the one needing to "break out". As a solution to this, it
> *might* be a good idea to add a mechanism to "force" absolute paths,
> even when the module_root is not None, for example by using a leading
> "/", as in "/turbogears.identity.User" (or whatever the real path is).
always treated as "real" absolute path, ignoring the "chroot" done by
module_root option.
Please ignore my yesterday's patch it's buggy. At the moment I'm working
on a new patch, to implemented the discussed features and give back
better error messages. I'm wondering if the entity argument to the
resolve function really has to be optional, as it is in the original
function. This blows up code quite a lot. When you want to give back as
much information as available in the error messages, you have to check
whether there was an entity provided or not.
Johannes
Gaetan de Menten
(r515). Also see my comments on the ticket.