DescriptorFields status/Manager API change
Joseph Kocherhans
soon? If not, I can give it a shot.
I'm not entirely clear as to how to do the ManyToMany and the
auto-generated end of the ForeignKey fields yet. It really feels like
they should just return a Manager instance (that automatically filters
on the original object's id) when they are accessed as an attribute of
an object, and that the Manager should (at least partially) adopt the
proposed descriptor API.
At any rate, Managers and ManyToMany/OneToMany attributes do extremely
similar things, and I think they could share an api and maybe even
share most implementation. If managers behaved like object attributes,
this is what I'm worried about:
MyModel.objects.filter(creator__exact=5)
for obj in MyModel.objects:
print obj
It seems like people might want to use the objects attribute with
different filter/ordering criteria multiple times. Some sort of
.clone() method would help. In the simple case you just use
MyModel.objects directly, but if you want different result sets, use
.clone()
object_set1 = MyModel.objects.clone()
object_set2 = MyModel.objects.clone()
object_set1.filter(creator__exact=1)
object_set2.filter(creator__exact=2)
Maybe unifying the attribute and manager api's isn't worth the
trouble. I'm sure there are some issues I'm overlooking here, so
please point out any problems you see.
Joseph
Jason Davies
Joseph Kocherhans wrote:
> At any rate, Managers and ManyToMany/OneToMany attributes do extremely
> similar things, and I think they could share an api and maybe even
> share most implementation. If managers behaved like object attributes,
> this is what I'm worried about:
>
> MyModel.objects.filter(creator__exact=5)
> for obj in MyModel.objects:
> print obj
I was under the impression that you'd do something like:
for obj in MyModel.objects.filter(creator__exact=5):
print obj
i.e. .filter() doesn't do anything to the existing MyModel.objects, but
returns a new lazy collection object with extra lookup params. Thus
you could do:
object_set1 = MyModel.objects.filter(creator__exact=1)
object_set2 = MyModel.objects.filter(creator__exact=2)
Regards,
Jason
Joseph Kocherhans
>
>
> Joseph Kocherhans wrote:
>
> > At any rate, Managers and ManyToMany/OneToMany attributes do extremely
> > similar things, and I think they could share an api and maybe even
> > share most implementation. If managers behaved like object attributes,
> > this is what I'm worried about:
> >
> > MyModel.objects.filter(creator__exact=5)
> > for obj in MyModel.objects:
> > print obj
>
> I was under the impression that you'd do something like:
>
> for obj in MyModel.objects.filter(creator__exact=5):
> print obj
If this were the case then it would be impossible to combine filter
and order_by, at least in this way:
MyModel.objects.filter(creator__exact=5)
MyModel.objects.order_by('status')
for obj in MyModel.objects:
print obj
For that code it seems like MyModel.objects should be a lazy
collection containing objects with creator=5 and ordered by status,
but in this case you would get an error because MyModel.objects
wouldn't support iteration.
If MyModel.objects (rather than MyModel.objects.filter()) instantiated
and returned a lazy collection, something like this should work as
expected:
for obj in MyModel.objects.filter(creator__exact=5).order_by('status'):
print obj
Or this:
myobjects = MyModel.objects
myobjects.filter(creator__exact=5)
myobjects.order_by('status')
for obj in myobjects:
print obj
Also, the current proposal doesn't apply to manipulators, just fields.
For the above you would still use managers like this:
MyModel.object.get_list(creator__exact=5, order_by=('status'))
Joseph
Adrian Holovaty
> Is anyone planning on tackling the descriptor fields proposal anytime
> soon? If not, I can give it a shot.
I was planning on starting this last night but got wrapped up in other
stuff. I'd like to start on it myself later this evening.
> MyModel.objects.filter(creator__exact=5)
> for obj in MyModel.objects:
> print obj
If managers behaved like that, how would one do the equivalent of get_object()?
Adrian
--
Adrian Holovaty
holovaty.com | djangoproject.com | chicagocrime.org
Joseph Kocherhans
> >
> >
> > Joseph Kocherhans wrote:
> >
> > > At any rate, Managers and ManyToMany/OneToMany attributes do extremely
> > > similar things, and I think they could share an api and maybe even
> > > share most implementation. If managers behaved like object attributes,
> > > this is what I'm worried about:
> > >
> > > MyModel.objects.filter(creator__exact=5)
> > > for obj in MyModel.objects:
> > > print obj
> >
> > I was under the impression that you'd do something like:
> >
> > for obj in MyModel.objects.filter(creator__exact=5):
> > print obj
>
> If this were the case then it would be impossible to combine filter
> and order_by, at least in this way:
>
> MyModel.objects.filter(creator__exact=5)
> MyModel.objects.order_by('status')
> for obj in MyModel.objects:
> print obj
>
> For that code it seems like MyModel.objects should be a lazy
> collection containing objects with creator=5 and ordered by status,
> but in this case you would get an error because MyModel.objects
> wouldn't support iteration.
Oops... MyModel.object would return an unfiltered/unordered lazy
collection. My mistake.
Joseph
Adrian Holovaty
> and order_by, at least in this way:
>
> MyModel.objects.filter(creator__exact=5)
> MyModel.objects.order_by('status')
> for obj in MyModel.objects:
> print obj
>
> For that code it seems like MyModel.objects should be a lazy
> collection containing objects with creator=5 and ordered by status,
> but in this case you would get an error because MyModel.objects
> wouldn't support iteration.
This is different than what I'd been envisioning -- I was under the
impression MyModel.objects wouldn't contain state. A manager shouldn't
contain state of the filter() queries that were passed to it. (This is
a good argument for treating managers differently than many-to-one and
many-to-many QueryResult objects.)
Joseph Kocherhans
>
> On 1/25/06, Joseph Kocherhans <jkoch...@gmail.com> wrote:
> > Is anyone planning on tackling the descriptor fields proposal anytime
> > soon? If not, I can give it a shot.
>
> I was planning on starting this last night but got wrapped up in other
> stuff. I'd like to start on it myself later this evening.
>
> > MyModel.objects.filter(creator__exact=5)
> > for obj in MyModel.objects:
> > print obj
>
> If managers behaved like that, how would one do the equivalent of get_object()?
Probably just MyModel.objects.get(1) where the first arg is assumed to
be the pk. You could also pass in slug__exact='test' or whatever.
Joseph
Joseph Kocherhans
Why should field attributes contain filter state, and not managers
though? (Probably neither of them should BTW.) Allowing
filters/ordering on attributes seems like a really convenient thing,
but I think the semantics of attribute access are getting in the way
here. At any rate, having managers work one way, and "to-many"
attributes another is going to be confising to people.
Joseph
Luke Plant
> If managers behaved like that, how would one do the equivalent of
> get_object()?
... and get_values() (which doesn't fit the 'sets' paradigm at all,
since it can contain duplicates)?
The original proposal was only for model instance attributes as far
as I understood it. This does bring up a problem with the proposal.
The similarity between
Foo.objects.get_list()
and
foo.get_bar_list()
makes it strange and confusing for them to use different syntax, (as
Joseph pointed out), but is it possible for them to use the same syntax?
You would have to do away with anything that isn't 'set'-like.
(I was just about to post something about the get_values() method and
sets, until I realised the original proposal didn't include the manager
methods, but I'm glad to see my nagging thoughts did have something
behind them!).
Luke
--
"I have had a perfectly lovely evening. However, this wasn't it."
(Groucho Marx)
Luke Plant || L.Plant.98 (at) cantab.net || http://lukeplant.me.uk/
Luke Plant
... also, what happens about exceptions i.e. when do they get thrown?
This applies to all the lazy collections, and also foreign key relationships:
e.g. from the wiki:
> article.reporter
> article.reporter.id (Doesn't do a DB query)
The first could throw Reporter.DoesNotExist, but the second doesn't.
I'm not sure that is a great plan really - and similar things apply to
the collections -- you have to start putting exception handling in
strange places, or round bigger blocks. I think this needs thinking
through properly too before any work starts. (I haven't thought these
things through - I'm just flagging up some worries before I rush out
for the evening).
Adrian Holovaty
http://code.djangoproject.com/wiki/DescriptorFields
It turns out related-object lookup can cleanly use the manager API.
Thoughts? If there are no big objections, let's start converting the
magic-removal unit tests to use this new syntax, and I'll start
implementation.
Joseph Kocherhans
>
> http://code.djangoproject.com/wiki/DescriptorFields
>
> It turns out related-object lookup can cleanly use the manager API.
> Thoughts? If there are no big objections, let's start converting the
> magic-removal unit tests to use this new syntax, and I'll start
> implementation.
I'm much happier with this. Good work Adrian!
One question though... it doesn't say anything about accessing fields
via the class anymore. Is that still the plan? Article.sites,
Article.reporter, etc would return the field object, and
article_obj.site_set returns a manager.
Also, it would be cool if the _set stuff wasn't forced on us, there's
a disconnect between creating an attribute called 'sites' in your
model, and accessing that attribute via 'site_set'. Allowing users to
set the attribute name on both sides would require some trickery, but
it could be done. (I can think of a couple of ways.) That said, I can
live with the '_set' names.
Joseph
Adrian Holovaty
> via the class anymore. Is that still the plan? Article.sites,
> Article.reporter, etc would return the field object, and
> article_obj.site_set returns a manager.
Yeah, definitely -- that's still planned. I was a bit delete-happy in
editing that wiki page. :)
> Also, it would be cool if the _set stuff wasn't forced on us, there's
> a disconnect between creating an attribute called 'sites' in your
> model, and accessing that attribute via 'site_set'. Allowing users to
> set the attribute name on both sides would require some trickery, but
> it could be done. (I can think of a couple of ways.) That said, I can
> live with the '_set' names.
Good call about the "sites" thing. Really, that attribute name isn't
used at all (from what I can tell)...hmmm. What are your ideas?
Joseph Kocherhans
Either pass in the attribute name of the related object (or as I'd
like to call it, "the ugly way"):
class Article(models.Model):
sites = models.ManyToManyField(Site, related_attr='articles')
Or, steal an idea from SQLObject and allow strings rather class names
for OneToOne, ManyToMany, and ForeignKey (I think internally,
SQLObject uses the string to lookup the object from inside the current
module, but it's been awhile since I looked at it):
class Article(models.Model):
sites = models.ManyToManyField('Site')
That way people *can* define both sides with plain old attribute
assignment, but they don't *have* to. If someone *didn't* specify one
of the attributes, it should get created anyhow using the '_set' name.
This option would make the most sense if there was one more field type
that could define the 'other' side of a ForeignKey. Finding a decent
name for that field type might be challenging though ;)
Joseph
Jacob Kaplan-Moss
> I've written up my latest proposal here:
>
> http://code.djangoproject.com/wiki/DescriptorFields
Yum!
I agree with Joseph -- I like this new syntax a *lot*.
I do have a few semantic questions, though. WARNING: Much nit-
picking follows; I should lead with the disclaimer that I'm *very*
happy with the syntax; I just want to make sure we get it as perfect
as possible!
First:
> Also, it would be cool if the _set stuff wasn't forced on us, there's
> a disconnect between creating an attribute called 'sites' in your
> model, and accessing that attribute via 'site_set'.
I *very* much agree with this.
Consider::
class Article(models.Model):
headline = models.CharField(maxlength=50)
reporter = models.ForeignKey(Reporter)
sites = models.ManyToManyField(Site)
The fact that I can't do ``article.sites.all()`` seems wrong -- is
there a reason for the foo_set name over the given field name that
I'm missing? This also deals with the edge case where I have
something like::
class Foo(models.Model):
bars = models.ManyToManyField(Bar)
bar_set = models.CharField()
Second, I'm still unhappy that it's not clear when a database lookup
is performed. Luckily, I think the proposed syntax makes it easy to
write a few rules so that you'll always know when the db is being
hit. How does this sound:
1. ``Manager.get`` always perfoms a lookup.
2. Any other manager method only actually hits the database when
iterated.
3. ``object.foreign_key`` always performs a lookup.
4. ``object.m2m_set`` is esentially a manager, so it behaves as
for 1 and 2.
There is a question of how/weather to cache database lookups::
article = Article.objects.get(pk=1)
article.reporter.fname # obvious performs a lookup
article.reporter.lname # does this?
I'd expect that subsequent accesses to the same foreign key do not
perform additional lookups, but if that's so there needs to be a way
to clear the cached object in case you have an object you need to
hang on to for a while. I'd suggest ``article.clear_relation_cache()``.
There's also the question of caching many to many lookups; my gut is
that Django shouldn't::
list(article.site_set) # hits the db
list(article.site_set) # does it again
Finally, it seems like the manager has a lot of methods, and I think
some of them are redundant. For one, ``Reporter.objects.all
().distinct()`` strikes me as less obvious than ``Reporter.objects.all
(distinct=True)``
In fact, I have trouble understanding why we need ``all()`` at all,
actually; what's the difference between these two?
::
Reporter.objects.all()
Reporter.objects.filter()
Why not rename "filter" to "list" (or "iterator" if we want to be
pedantic) and collapse the two methods into one?
Similarly, the chaining of filter/order_by/in_bulk seems crufty to
me; are these equivalent?
::
Reporter.objects.filter(fname="Joe").order_by("fname")
Reporter.objects.order_by("fname").filter(fname="Joe")
I'm a big fan of the "One True Way" principle, so that rubs me the
wrong way. Why not simply have order_by/distinct/etc. be kwargs to
the manager functions? If there's a good reason to do the "chained
methods" that's fine, but let's not be too clever for our own sakes, eh?
Adrian, thanks again for thinking this through -- I think this change
will make code a *lot* more readable, and of course a lot more fun.
Jacob
hugo
>wrong way. Why not simply have order_by/distinct/etc. be kwargs to
>the manager functions? If there's a good reason to do the "chained
>methods" that's fine, but let's not be too clever for our own sakes, eh?
One nice thing about chained methods is, you can pass around the query
object and modify it after the fact. Think "curried queries" - you
build a partial query and pass it to a function, which itself can add
more query specifications or ordering or stuff like that, without
needing to know about what the actual query is. This opens up a really
nice way to build generic code: you only need to put those query
specifications into your code that _your_ code needs, any filtering for
example can be done outside.
And I think all/filter is quite nice, too - that way you can see
directly whether a query starts with the full set or wether you work on
a subset. Sure, this can be done with one single method with
parameters, but I rather like the distinguished method names - it's
more readable to me.
bye, Georg
hugo
>used at all (from what I can tell)...hmmm. What are your ideas?
Regardless of what you do in the end: please no "magic name invention",
especially if it is something like "attribute sites becomes site_set" -
that way we would be back in magic pluralization/singularization land
;-)
Is there any reason why the article_obj.sites attribute can't hold the
manager already? We never directly access that attribute anyway - as it
doesn't carry any value of meaning. So Article.sites would be the
ManyToMany field and article_obj.sites would be the manager - looks
nice to me. That way you can just do article_obj.sites.all() to get all
linked objects. If people want to have the _set in the name, they can
allways just name the attribute "site_set" instead of "sites".
bye, Georg
Robert Wittams
Any reason here why Manager couldn't inherit from Query (or a common
base type, eg QuerySet ), so it can be used directly as well as/instead
of via .all() ?
Also, you need to mention that Query instances can be combined via & and
| ( and support the other set stuff), but that the results are lazily
calculated.
Jacob Kaplan-Moss
> One nice thing about chained methods is, you can pass around the query
> object and modify it after the fact. Think "curried queries" - you
> build a partial query and pass it to a function, which itself can add
> more query specifications or ordering or stuff like that, without
> needing to know about what the actual query is. This opens up a really
> nice way to build generic code: you only need to put those query
> specifications into your code that _your_ code needs, any filtering
> for
> example can be done outside.
Ah, interesting -- I hadn't thought about that situation. So
essentially a Query is a curried lookup until you iterate it, yes?
What happens to a query after it's been iterated? For example, how
does the following behave?
::
people = Reporter.objects.filter(fname="Joe")
for p in people:
print p
people2 = people.order_by('fname')
for p in people:
print p
> And I think all/filter is quite nice, too - that way you can see
> directly whether a query starts with the full set or wether you
> work on
> a subset. Sure, this can be done with one single method with
> parameters, but I rather like the distinguished method names - it's
> more readable to me.
Fair enough -- my tastes run the other way towards a single method,
but it's really not the big a deal.
Jacob
Adrian Holovaty
> > Also, it would be cool if the _set stuff wasn't forced on us, there's
> > a disconnect between creating an attribute called 'sites' in your
> > model, and accessing that attribute via 'site_set'.
>
> I *very* much agree with this.
>
> Consider::
>
> class Article(models.Model):
> headline = models.CharField(maxlength=50)
> reporter = models.ForeignKey(Reporter)
> sites = models.ManyToManyField(Site)
>
> The fact that I can't do ``article.sites.all()`` seems wrong -- is
> there a reason for the foo_set name over the given field name that
> I'm missing? This also deals with the edge case where I have
> something like::
The reason is consistency. If we enforce the "_set" thing (or whatever
other name we come up with), that means there's a simple rule to
remember:
* To access related objects, whether they're one-to-many or
many-to-many, just use the attribute called "modelname_set" on your
object, where modelname is the lowercase name of the related model.
If, instead, we allow the article.sites thing (instead of
article.site_set), that introduces a special-case for many-to-many
relationships.
Make sense? But, yeah, I realize that the "sites" attribute name in
that model is essentially meaningless. Any other ideas?
> Second, I'm still unhappy that it's not clear when a database lookup
> is performed. Luckily, I think the proposed syntax makes it easy to
> write a few rules so that you'll always know when the db is being
> hit. How does this sound:
>
> 1. ``Manager.get`` always perfoms a lookup.
> 2. Any other manager method only actually hits the database when
> iterated.
> 3. ``object.foreign_key`` always performs a lookup.
> 4. ``object.m2m_set`` is esentially a manager, so it behaves as
> for 1 and 2.
I was thinking this would behave exactly as before --
1. Manager.get() always performs a lookup.
2. Any other default manager method only actually hits the
database when iterated (or repr()'d?).
3. object.foreign_key performs a lookup only the first time --
unless select_related=True was used on the lookup of the parent
object, in which case the value would already be cached.
4. object.m2m_set is esentially a manager, so it behaves as for 1 and 2.
> There is a question of how/weather to cache database lookups::
>
> article = Article.objects.get(pk=1)
> article.reporter.fname # obvious performs a lookup
> article.reporter.lname # does this?
No, that second one wouldn't perform a lookup. This would work exactly
as before, just like article.get_reporter().fname and
article.get_reporter().lname -- it's cached the first time it's
accessed.
> I'd expect that subsequent accesses to the same foreign key do not
> perform additional lookups, but if that's so there needs to be a way
> to clear the cached object in case you have an object you need to
> hang on to for a while. I'd suggest ``article.clear_relation_cache()``.
We haven't had this up to this point, but -- sure. :)
> There's also the question of caching many to many lookups; my gut is
> that Django shouldn't::
>
> list(article.site_set) # hits the db
> list(article.site_set) # does it again
I believe Django currently *does* cache many-to-many lookups, so I'd
been thinking it would continue to do so -- but I don't really mind
either way.
> Finally, it seems like the manager has a lot of methods, and I think
> some of them are redundant. For one, ``Reporter.objects.all
> ().distinct()`` strikes me as less obvious than ``Reporter.objects.all
> (distinct=True)``
Two reasons for making distinct a method instead of a keyword argument --
1. As Hugo pointed out, this makes it possible to chain queries.
2. It makes it possible to leave the "__exact" off, because we can be
positive any keyword argument to filter() is a field lookup, not a
meta argument such as "distinct" or "limit". For example, if you have
a field name called "distinct", you'd be able to do
Reporter.objects.filter(distinct='blah'), which would be the
equivalent of Reporter.objects.filter(distinct__exact='blah').
Granted, this is more of a side benefit than a reason *to* do it, but
I like it because it lets us trim the "__exact", which is the most
common lookup type.
> In fact, I have trouble understanding why we need ``all()`` at all,
> actually; what's the difference between these two?
>
> Reporter.objects.all()
> Reporter.objects.filter()
>
> Why not rename "filter" to "list" (or "iterator" if we want to be
> pedantic) and collapse the two methods into one?
It's for readability, like Hugo said, but I'm not 100% sold on it
either way. Come to think of it, get() accepts filter arguments, so,
if we have a list(), it'd be nice and consistent if list() also
accepted filter arguments.
> Similarly, the chaining of filter/order_by/in_bulk seems crufty to
> me; are these equivalent?
>
> Reporter.objects.filter(fname="Joe").order_by("fname")
> Reporter.objects.order_by("fname").filter(fname="Joe")
>
> I'm a big fan of the "One True Way" principle, so that rubs me the
> wrong way. Why not simply have order_by/distinct/etc. be kwargs to
> the manager functions? If there's a good reason to do the "chained
> methods" that's fine, but let's not be too clever for our own sakes, eh?
Again, it's all about chaining the methods to be able to pass around
the query object, and limiting the filter() arguments to remove the
"meta" arguments.
Tim Keating
> One nice thing about chained methods is, you can pass around the query
> object and modify it after the fact. Think "curried queries" - you
> build a partial query and pass it to a function, which itself can add
> more query specifications or ordering or stuff like that, without
> needing to know about what the actual query is. This opens up a really
> nice way to build generic code: you only need to put those query
> specifications into your code that _your_ code needs, any filtering for
> example can be done outside.
>
> And I think all/filter is quite nice, too - that way you can see
> directly whether a query starts with the full set or wether you work on
> a subset. Sure, this can be done with one single method with
> parameters, but I rather like the distinguished method names - it's
> more readable to me.
+1 to this argument from me. I, too, like "all/filter" -- explicit is
better than implicit.
TK
Jacob Kaplan-Moss
> The reason is consistency. If we enforce the "_set" thing (or whatever
> other name we come up with), that means there's a simple rule to
> remember:
> Make sense? But, yeah, I realize that the "sites" attribute name in
> that model is essentially meaningless. Any other ideas?
It seems to me that the translation from ``sites`` in the model to
``site_set`` in the instance is "worse" than having an inconsistency
between m2m and o2m relations. In fact, I don't really have a
problem with the two relation types behaving differently.
I'd suggest that m2m relations use the attribute name, and that we
introduce an option for ForeignKey that lets you override the o2m name::
class Article:
writer = meta.ForeignKey(Reporter, related_name="articles")
sites = meta.ManyToManyField(Site)
s = Site.objects.get(pk=1)
r = Reporter.objects.get(pk=1)
s.article_set.all()
r.articles.all()
That is, if ``related_name`` isn't given then ``OBJECT_set`` is used.
<impression voice="DHH">Convention over Configuration!</impression>
> I was thinking this would behave exactly as before --
>
> 1. Manager.get() always performs a lookup.
> 2. Any other default manager method only actually hits the
> database when iterated (or repr()'d?).
> 3. object.foreign_key performs a lookup only the first time --
> unless select_related=True was used on the lookup of the parent
> object, in which case the value would already be cached.
> 4. object.m2m_set is esentially a manager, so it behaves as for
> 1 and 2.
Perfect.
>> I'd expect that subsequent accesses to the same foreign key do not
>> perform additional lookups, but if that's so there needs to be a way
>> to clear the cached object in case you have an object you need to
>> hang on to for a while. I'd suggest ``article.clear_relation_cache
>> ()``.
>
> We haven't had this up to this point, but -- sure. :)
That's a good point; is it worth adding this method at all? Is anyone
actually going to need it?
> I believe Django currently *does* cache many-to-many lookups, so I'd
> been thinking it would continue to do so -- but I don't really mind
> either way.
I'm also OK either way, but let's make it clear (somewhere) which it is.
> Two reasons for making distinct a method instead of a keyword
> argument --
>
> 1. As Hugo pointed out, this makes it possible to chain queries.
> 2. It makes it possible to leave the "__exact" off, because we can be
> positive any keyword argument to filter() is a field lookup, not a
> meta argument such as "distinct" or "limit". For example, if you have
> a field name called "distinct", you'd be able to do
> Reporter.objects.filter(distinct='blah'), which would be the
> equivalent of Reporter.objects.filter(distinct__exact='blah').
> Granted, this is more of a side benefit than a reason *to* do it, but
> I like it because it lets us trim the "__exact", which is the most
> common lookup type.
Yeah, I'm convinced -- I had trouble reproducing your reasoning, but
now that you explain it it makes perfect sense.
> It's for readability, like Hugo said, but I'm not 100% sold on it
> either way. Come to think of it, get() accepts filter arguments, so,
> if we have a list(), it'd be nice and consistent if list() also
> accepted filter arguments.
That's kinda what I would expect. The only problem is that list()
doesn't actually return a list -- it returns a Query that can be
iterated over... Hm...
OK, what about this: make ``objects`` callable::
Before After
Article.objects.get_list() Article.objects()
Article.objects.get_list(**kw) Article.objects(**kw)
Article.objects.get_object(**kw) Article.objects.get(**kw)
Article.objects.get_values(**kw) Article.objects.values(**kw)
That's more concise in the common case -- I'd guess that get_list()
is far and away the most common method used -- and it does away with
the filter/all distinction.
Thoughts?
Jacob
Adrian Holovaty
> It seems to me that the translation from ``sites`` in the model to
> ``site_set`` in the instance is "worse" than having an inconsistency
> between m2m and o2m relations. In fact, I don't really have a
> problem with the two relation types behaving differently.
>
> I'd suggest that m2m relations use the attribute name, and that we
> introduce an option for ForeignKey that lets you override the o2m name::
>
> class Article:
> writer = meta.ForeignKey(Reporter, related_name="articles")
> sites = meta.ManyToManyField(Site)
>
> s = Site.objects.get(pk=1)
> r = Reporter.objects.get(pk=1)
>
> s.article_set.all()
> r.articles.all()
>
> That is, if ``related_name`` isn't given then ``OBJECT_set`` is used.
OK, this sounds good to me.
> OK, what about this: make ``objects`` callable::
>
> Before After
>
> Article.objects.get_list() Article.objects()
> Article.objects.get_list(**kw) Article.objects(**kw)
> Article.objects.get_object(**kw) Article.objects.get(**kw)
> Article.objects.get_values(**kw) Article.objects.values(**kw)
>
> That's more concise in the common case -- I'd guess that get_list()
> is far and away the most common method used -- and it does away with
> the filter/all distinction.
The problem with this is that Article.objects is a Manager instance,
so it would be slightly ugly and special-casish to have to specify
behavior for __call__(). If you wanted to override the functionality
for a custom manager, you'd have to override __call__(). That's not
"magic" per se, but it's still a bit of a special case. Gotta say I
really like the all() and filter() explicitness.
Robert Wittams
> class Article:
> writer = meta.ForeignKey(Reporter, related_name="articles")
> sites = meta.ManyToManyField(Site)
>
> s = Site.objects.get(pk=1)
> r = Reporter.objects.get(pk=1)
>
> s.article_set.all()
> r.articles.all()
>
> That is, if ``related_name`` isn't given then ``OBJECT_set`` is used.
Given that this is identical to my original plan for descriptor fields (
don't randomly rename things, only make up names when no name is
provided by the user), I'm all for this. Making <whatever>_set into
religion was not the aim...
>
> That's kinda what I would expect. The only problem is that list()
> doesn't actually return a list -- it returns a Query that can be
> iterated over... Hm...
>
> OK, what about this: make ``objects`` callable::
>
> Before After
>
> Article.objects.get_list() Article.objects()
> Article.objects.get_list(**kw) Article.objects(**kw)
> Article.objects.get_object(**kw) Article.objects.get(**kw)
> Article.objects.get_values(**kw) Article.objects.values(**kw)
>
> That's more concise in the common case -- I'd guess that get_list() is
> far and away the most common method used -- and it does away with the
> filter/all distinction.
>
> Thoughts?
I don't really like it. I much prefer the idea of the manager being
something you can iterate over directly, and filter being a method to
use when you want to filter it, that returns an object of the same type
but just filtered down. Ie Article.objects is just a set that happens to
be backed by the database, and just happens to be filterable. Seems
pretty intuitive to me. No .all() or __call__ required.
I don't think the same can be said for making random objects callable...
I have no idea what this means, its just entirely arbitrary. Things
should generally only pretend to be functions when that is their main
purpose.
luke....@gmail.com
Article.objects, then surely Article.objects is already a collection of
some kind. That's what is sounds like: Article.objects == all the
'Article' objects.
Also, if Query instances act as sets, they should support len(), and
you don't need .count() :
reporter_obj.get_article_count() -> len(reporter_obj.article_set)
Article.objects.get_count() -> len(Article.objects)
This, too, makes much more sense without the .all()
Luke
luke....@gmail.com
If the related objects lookups always use the corresponding manager,
then this:
> article_obj.reporter
would be equivalent to:
> Reporter.objects.get(pk=article_obj.id)
That can throw a Reporter.DoesNotExist exception, which might not just
be corner case i.e. it might be perfectly allowable in your model to
have articles with a null reporter.
In the proposal (the original one at least), article_obj.reporter.id
wouldn't do a DB lookup, so .reporter must be lazy, which kind of makes
things worse - you can use reporter.id, but reporter.name will blow up
on you.
There are two use cases AFAICS:
- in view code, you want to know immediately if the object doesn't
exist, and not have to do some silly tricks to get the lazy object to
initialise from the db and throw any appropriate exceptions
- in template code, you might want:
{% if article_obj.reporter %}
Written by {{ article_obj.reporter.name }}
{% endif %}
Can we do both these? Perhaps a .lazyget() method on objects will help
- foreign key fields always translate into that. The __get__()
descriptor on the article_obj.reporter always does a .lazyget(), which
returns a lazy object which only has the id set. This way we can still
do article_obj.reporter.id without DB access. The lazy object needs a
__nonzero__ method that will first initialise it, and if it fails to
initialise it then return false, otherwise true.
.get(), on the other hand, doesn't get a lazy object, but immediately
throws exceptions if it's not there.
Think that covers it - are there any holes?
Luke
Joseph Kocherhans
>
> I'm not a fan of .all() either - if you can do .filter() on
> Article.objects, then surely Article.objects is already a collection of
> some kind. That's what is sounds like: Article.objects == all the
> 'Article' objects.
And there's the original problem that started this mess ;-) Let me see
if I can lay it all out. (Sorry to pick on your response Luke, not
trying to single you out.)
Case 1:
Let's assume that MyModel.objects or my_object.related_set *IS* a
Query object (in other words, it has state):
MyModel.objects # returns Q1
MyModel.objects.filter(*args) # Q1 with filters applied
q = MyModel.objects # q is Q1
q.filter(*args) # q is still Q1, but with filters now
q.order_by('test') # q is still Q1 + filters + ordering
q2 = MyModel.objects # oh shit, this is Q1 + filters + ordering, but I
expected a new Q
If you want to get 2 different iterators with different filter
criteria, you need some sort of .clone() method on Query objects.
q2 = MyModel.objects.clone() # ahhh... a new Q :)
Case 2:
Let's assume that MyModel.objects and my_object.related_set *RETURN* a
*NEW* Query object. (In other words is stateless.) You might expect
this:
MyModel.objects # Q1
MyModel.objects.filter(*args) # Q1 with filters applied
MyModel.objects.order_by(*args) # Q1 with ordering applied
But in fact, you'll get this:
MyModel.objects # Q1
MyModel.objects.filter(*args) # Q2
MyModel.objects.order_by(*args) # Q3
But this works fine:
q = MyModel.objects # q is Q1
q.filter(*args) # q is Q1 with filters applied
q.order_by(*args) # q is Q1 with ordering applied
My point was that you have to explain the difference between the last
two cases, or if MyModel.objects is stateful (as in Case 1) you need
that clone method I talked about. (Also, non-intuitive)
The syntax Robert proposed is very appealing to me, but AFAICS, the
facts that follow from that syntax are confusing as hell. If I'm
missing something, please let me know.
> Also, if Query instances act as sets, they should support len(), and
> you don't need .count() :
Ian Bicking has kept len() out of SQLObject result sets even though it
seems really intuitive to use. Here's a rundown of what I remember
about his argument: __len__ would run "count (*)" against the db. I
think iter() calls len() implicitly for performance reasons, so you'd
be running a useless count(*) every time you started iterating over a
Query object. On the other hand, maybe if the iterator has already
cahced the objects, __len__ could just call len() on the cache. It
might be possible to work something out.
Joseph
Robert Wittams
> On 1/26/06, luke....@gmail.com <luke....@gmail.com> wrote:
>
>>I'm not a fan of .all() either - if you can do .filter() on
>>Article.objects, then surely Article.objects is already a collection of
>>some kind. That's what is sounds like: Article.objects == all the
>>'Article' objects.
>
>
> And there's the original problem that started this mess ;-) Let me see
> if I can lay it all out. (Sorry to pick on your response Luke, not
> trying to single you out.)
>
> Case 1:
> Let's assume that MyModel.objects or my_object.related_set *IS* a
> Query object (in other words, it has state):
I have absolutely no idea how you arrived at this weirdo stateful model
- it clearly makes absolutely no sense wrt concurrency.
> Case 2:
> Let's assume that MyModel.objects and my_object.related_set *RETURN* a
> *NEW* Query object. (In other words is stateless.) You might expect
> this:
>
> MyModel.objects # Q1
> MyModel.objects.filter(*args) # Q1 with filters applied
> MyModel.objects.order_by(*args) # Q1 with ordering applied
>
> But in fact, you'll get this:
>
> MyModel.objects # Q1
> MyModel.objects.filter(*args) # Q2
> MyModel.objects.order_by(*args) # Q3
>
> But this works fine:
>
> q = MyModel.objects # q is Q1
> q.filter(*args) # q is Q1 with filters applied
> q.order_by(*args) # q is Q1 with ordering applied
I really have no idea what you are talking about here. Sorry.
The only interpretation I can make is that you are worried about making
multiple database calls when you . The only alternative to this is to
reimplement a relational database in python. Do you really want to do
the filtering client side? Or what? I am having real trouble grasping
your issue here.
Luke Plant
> Let's assume that MyModel.objects or my_object.related_set *IS* a
> Query object (in other words, it has state):
It's state is simply an empty set of where clauses and an empty order by
list and these never actually change.
> q = MyModel.objects # q is Q1
> q.filter(*args) # q is still Q1, but with filters now
No, q.filter(*args) is a *new* set, it doesn't change q
> q.order_by('test') # q is still Q1 + filters + ordering
Again, a new set, which doesn't include the filter at all. The clone
would be done by the filter() and order_by() methods.
Unless I'm missing something about why that isn't possible?
> Ian Bicking has kept len() out of SQLObject result sets even though
> it seems really intuitive to use. Here's a rundown of what I remember
> about his argument: __len__ would run "count (*)" against the db. I
> think iter() calls len() implicitly for performance reasons, so you'd
> be running a useless count(*) every time you started iterating over a
> Query object. On the other hand, maybe if the iterator has already
> cahced the objects, __len__ could just call len() on the cache. It
> might be possible to work something out.
I'm happy with that, but I've done some quick tests and it doesn't seem
that __len__ is called when you get or use the iterator.
The only issue I've come across is that since managers are used to do
related object lookups, and related objects are cached, so are all
queries done by the manager - so iterating over Article.objects will
retrieve all the articles and keep them in a cache. But I imagine some
weakrefs will be adequate to sort that out.
Luke
--
I heard a man say that brigands demand your money or your life, whereas
women require both. (Samuel Butler)
Luke Plant || L.Plant.98 (at) cantab.net || http://lukeplant.me.uk/
--
"I imagine bugs and girls have a dim suspicion that nature played a
cruel trick on them, but they lack the intelligence to really
comprehend the magnitude of it." (Calvin and Hobbes)
Joseph Kocherhans
>
> Joseph Kocherhans wrote:
> > On 1/26/06, luke....@gmail.com <luke....@gmail.com> wrote:
> >
> >>I'm not a fan of .all() either - if you can do .filter() on
> >>Article.objects, then surely Article.objects is already a collection of
> >>some kind. That's what is sounds like: Article.objects == all the
> >>'Article' objects.
> >
> >
> > And there's the original problem that started this mess ;-) Let me see
> > if I can lay it all out. (Sorry to pick on your response Luke, not
> > trying to single you out.)
> >
> > Case 1:
> > Let's assume that MyModel.objects or my_object.related_set *IS* a
> > Query object (in other words, it has state):
>
> I have absolutely no idea how you arrived at this weirdo stateful model
> - it clearly makes absolutely no sense wrt concurrency.
Wow. I'm not even sure how I got to that point now.
> > Case 2:
> > Let's assume that MyModel.objects and my_object.related_set *RETURN* a
> > *NEW* Query object. (In other words is stateless.) You might expect
> > this:
> >
> > MyModel.objects # Q1
> > MyModel.objects.filter(*args) # Q1 with filters applied
> > MyModel.objects.order_by(*args) # Q1 with ordering applied
> >
> > But in fact, you'll get this:
> >
> > MyModel.objects # Q1
> > MyModel.objects.filter(*args) # Q2
> > MyModel.objects.order_by(*args) # Q3
> >
> > But this works fine:
> >
> > q = MyModel.objects # q is Q1
> > q.filter(*args) # q is Q1 with filters applied
> > q.order_by(*args) # q is Q1 with ordering applied
>
> I really have no idea what you are talking about here. Sorry.
>
> The only interpretation I can make is that you are worried about making
> multiple database calls when you . The only alternative to this is to
> reimplement a relational database in python. Do you really want to do
> the filtering client side? Or what? I am having real trouble grasping
> your issue here.
Nevermind. I was making an (erroneous and old) assumption that you
could modify the state of a query instance like:
q.filter()
q.order_by()
When in fact Adrian's proposal specifically says that filter() and
order_by() would return new query instances, not modify q's state (a
dict of filter criteria or whatever) I'm sorry :( To do the above, you
would have to chain the methods like:
q.filter().order_by()
or reassign q
q = q.filter()
q = q.order_by()
I'm happy now. +1 on MyModel.objects -> new Query instance rather than
MyModel.objects.all()
Sorry for the confusion. Hopefully this clears things up for someone
else as well.
Joseph
hugo
>essentially a Query is a curried lookup until you iterate it, yes?
Yep.
>What happens to a query after it's been iterated? For example, how
>does the following behave?
I'd say it should memoize it's result - so it is only queried once per
request to reduce database hits. But there should maybe a .reset()
method on a manager to reset the memoized data, so you can rerun your
query if you need to (won't happen that often in web code, but might be
needed in batch code).
bye, Georg
Russell Keith-Magee
> I'd suggest that m2m relations use the attribute name, and that we
> introduce an option for ForeignKey that lets you override the o2m name::
>
> class Article:
> writer = meta.ForeignKey(Reporter, related_name="articles")
> sites = meta.ManyToManyField(Site)
>
> s = Site.objects.get(pk=1)
> r = Reporter.objects.get(pk=1)
>
> s.article_set.all()
> r.articles.all()
>
> That is, if ``related_name`` isn't given then ``OBJECT_set`` is used.
Two quick notes:
1) ManyToManyField will need a related_name argument, not just o2m
fields, so as to handle reverse direction queries (e.g.,
sites.article_set). This may have been implied, but so far all the
disussion and all the examples have been about ForeignKey renaming.
2) I have a minor problem with the _set suffix: to me, _set implies
uniqueness in the returned results, which will not exist unless
.distinct()/distinct=True is used. Either distinct needs to be turned
on by default and disabled by parameter/filter (which I have argued
for in a previous thread), or a suffix that does not imply uniqueness
is required. Possible candidates: _list, _objects
Russ Magee %-)
Robert Wittams
> 2) I have a minor problem with the _set suffix: to me, _set implies
> uniqueness in the returned results, which will not exist unless
> .distinct()/distinct=True is used. Either distinct needs to be turned
> on by default and disabled by parameter/filter (which I have argued
> for in a previous thread), or a suffix that does not imply uniqueness
> is required. Possible candidates: _list, _objects
Could you provide an example where you would actually end up with
duplicate results here? I'm having trouble thinking of how this would
actually occur. AFAICT, each child object will only show up once in the
results. If there is one, I agree that it would make sense to use
distinct by default.
There was a huge long thread a while back in which other names were
discussed. Look it up in the archive if you like.
The main reason I like _set is that it suggests a bunch of operations
semantics that are fairly natural : that of the built-in set type in
Python 2.4 . _list is incredibly misleading (see the previous threads),
_objects is fairly vague...
Russell Keith-Magee
> >What happens to a query after it's been iterated? For example, how
> >does the following behave?
>
> I'd say it should memoize it's result - so it is only queried once per
> request to reduce database hits. But there should maybe a .reset()
> method on a manager to reset the memoized data, so you can rerun your
> query if you need to
Why not make the query object itself be the thing that is reset? e.g.
people = Reporter.objects.filter(fname="Joe")
for p in people:
print p
people.reset()
people = people.filter(lname="Smith")
for p in people:
print p
Would print all the reporters named Joe, reset the query, then rerun a
revised query to print all the reporters named Joe Smith.
Taking this approach a little further, it could also address Adrian's
Manager __call__ problem with Jacob's Article.objects() proposal.
Rather than exposing the manager itself, expose an interface that can
be used as a factory for producing Query objects. Keep the Manager
internally as a mechanism for managing database connections and SQL
composition, but don't expose it as the Article.objects member.
On the class itself, Article.objects(), Article.values(),
Article.in_bulk() become factory methods for producing Query objects
which, when iterated, provide objects of the expected type (instances,
dictionaries, etc).
filter, order_by, etc are kept as methods on a query object itself,
rather than methods on the manager. If you want to apply a filter, use
Article.objects().filter(headline="foo"). The metaphor here is 'make a
basic query object, then narrow it with a filter'.
On class instances, article_obj.sites() becomes the analogous factory
method for queries.
This approach also simplifies one use case for multiple managers -
pre-filtered Managers. If you need every query to have a particular
filter pre-applied, add a class method that returns
objects().filter(...) as required.
The one problem I can see is what to do with
article_obj.sites().clear(). I don't have a solution for this one,
other than to suggest making 'sites' an object that returns a query on
__call__. This is the same special case that Adrian objected to, but
it now only applies to modifying relationships in m2m queries, rather
than to every single class and query in the system.
Russ Magee %-)
Robert Wittams
> Taking this approach a little further, it could also address Adrian's
> Manager __call__ problem with Jacob's Article.objects() proposal.
> Rather than exposing the manager itself, expose an interface that can
> be used as a factory for producing Query objects. Keep the Manager
> internally as a mechanism for managing database connections and SQL
> composition, but don't expose it as the Article.objects member.
>
> On the class itself, Article.objects(), Article.values(),
> Article.in_bulk() become factory methods for producing Query objects
> which, when iterated, provide objects of the expected type (instances,
> dictionaries, etc).
>
> filter, order_by, etc are kept as methods on a query object itself,
> rather than methods on the manager. If you want to apply a filter, use
> Article.objects().filter(headline="foo"). The metaphor here is 'make a
> basic query object, then narrow it with a filter'.
>
> On class instances, article_obj.sites() becomes the analogous factory
> method for queries.
I've got to say, this is absolutely horrible and non-obvious. API design
should not be an exercise in how clever or confusing you should be - it
should be non-surprising. Article.objects acting as a set of Articles is
non-surprising. Article.objects.filter(whatever="fish") returning a new
filtered set is non surprising.
> This approach also simplifies one use case for multiple managers -
> pre-filtered Managers. If you need every query to have a particular
> filter pre-applied, add a class method that returns
> objects().filter(...) as required.
And this is simpler than
class Something:
name = CharField(maxlength=100)
objects = Manager()
bad_objects = objects.filter(name_startswith="bad")
how? (Yes, its a contrived example.)
Russell Keith-Magee
>
> Russell Keith-Magee wrote:
> > 2) I have a minor problem with the _set suffix: to me, _set implies
> > uniqueness in the returned results, which will not exist unless
> > .distinct()/distinct=True is used.
>
> duplicate results here? I'm having trouble thinking of how this would
> actually occur. AFAICT, each child object will only show up once in the
> results. If there is one, I agree that it would make sense to use
> distinct by default.
Any query involving joins over a m2o or m2m relation; if there are
multiple matches in the related table, there will be multiple rows
with the fields from the primary table: e.g.,
daily_planet = Newspaper.objects.get_object(name='Daily Planet')
daily_planet.reporter_set.filter(article_headline__startswith="Foo")
will duplicate results for every reporter that has written more than
one article with a headline starting with 'Foo'.
> There was a huge long thread a while back in which other names were
> discussed. Look it up in the archive if you like.
Ok. I missed that discussion the first time around. I now agree that
_set is the best of the bunch. Fixing distinct seems the better
approach.
Russ Magee %-)
Robert Wittams
> On 1/27/06, Robert Wittams <rob...@wittams.com> wrote:
>
>>Russell Keith-Magee wrote:
>>
>>>2) I have a minor problem with the _set suffix: to me, _set implies
>>>uniqueness in the returned results, which will not exist unless
>>>.distinct()/distinct=True is used.
>>
>>Could you provide an example where you would actually end up with
>>duplicate results here? I'm having trouble thinking of how this would
>>actually occur. AFAICT, each child object will only show up once in the
>>results. If there is one, I agree that it would make sense to use
>>distinct by default.
>
>
> Any query involving joins over a m2o or m2m relation; if there are
> multiple matches in the related table, there will be multiple rows
> with the fields from the primary table: e.g.,
>
> daily_planet = Newspaper.objects.get_object(name='Daily Planet')
> daily_planet.reporter_set.filter(article_headline__startswith="Foo")
>
> will duplicate results for every reporter that has written more than
> one article with a headline starting with 'Foo'.
I agree we don't want this current behaviour. I had some hazy idea that
this got turned into group by malarkeys - obviously not...
Yes, I think distinct is the right way to go here.
luke....@gmail.com
> Ian Bicking has kept len() out of SQLObject result sets even though it
> seems really intuitive to use. Here's a rundown of what I remember
> about his argument: __len__ would run "count (*)" against the db. I
> think iter() calls len() implicitly for performance reasons, so you'd
> be running a useless count(*) every time you started iterating over a
> Query object.
OK, I think I have tracked this down - or a related issue. This is
relevant whether we go with __len__ or not.
If you have this view code (updated for the new syntax):
context['articles'] = Article.objects # or Article.objects.all()
And this template (exactly as before the syntax change):
{% if articles %}
{% for article in articles %}
.. {{ article }}
{% endfor %}
{% else %}
Sorry, no articles
{% endif %}
With the new syntax method, you never get the 'Sorry no articles',
because Article.objects evaluates to true. This would be a bug with
the new syntax. However, if Article.objects or Articles.objects.all()
has a __len__ method as I proposed, then bool() will call it and return
false if the answer is zero. So the above code would work, but do two
db lookups, one for a count and the other for a list, which is also
different from before, and is a performance bug.
(NB I think this is 2.3/2.4 related - apparently the behaviour of
bool(iter([])) changed.)
One way around it would be to add __nonzero__ to the Query instance (I
think), which would get and cache the list of objects, ready to be
iterated. This would be slightly subtle behaviour, but something like
this needs to be done. So you would have:
bool(Article.objects) # retrieves list
iter(Article.objects) # retrieves list
len(Article.objects) # does count(*)
Luke
Russell Keith-Magee
> > Article.in_bulk() become factory methods for producing Query objects
> > which, when iterated, provide objects of the expected type (instances,
> > dictionaries, etc).
>
> should not be an exercise in how clever or confusing you should be - it
> should be non-surprising. Article.objects acting as a set of Articles is
> non-surprising. Article.objects.filter(whatever="fish") returning a new
> filtered set is non surprising.
It wasn't an attempt to be confusing. It was an attempt to overcome a
problem with the Article.objects notation - namely, when is a query
executed, and how do you reset it? And I don't grant that
Article.objects acting as a set is _completely_ non-surprising.
Consider Python dictionaries. If you have a dictionary, you call:
for key, value in my_dict.items(): ...
_not_
for key, value in my_dict.items:...
Why? Because my_dict.items looks like it's an attribute of my_dict -
which it isn't - its an iterator: a stateful entity unto itself, so it
is produced using a factory method on the dictionary.
Queries are even more stateful than iterators, because they can be
cached and reused, so they have the need to be both reset and
re-executed over the database. How does this cache/reuse match the use
expectations of a set as a standalone object?
Russ Magee %-)
Robert Wittams
> On 1/27/06, Robert Wittams <rob...@wittams.com> wrote:
>
>>>On the class itself, Article.objects(), Article.values(),
>>>Article.in_bulk() become factory methods for producing Query objects
>>>which, when iterated, provide objects of the expected type (instances,
>>>dictionaries, etc).
>>
>>I've got to say, this is absolutely horrible and non-obvious. API design
>>should not be an exercise in how clever or confusing you should be - it
>>should be non-surprising. Article.objects acting as a set of Articles is
>>non-surprising. Article.objects.filter(whatever="fish") returning a new
>>filtered set is non surprising.
>
>
> It wasn't an attempt to be confusing.
The intent does not change the effect.
> It was an attempt to overcome a
> problem with the Article.objects notation - namely, when is a query
> executed
When the results are needed? This is the entire basis of lazy evaluation.
> and how do you reset it?
I think this is a legitimate need, and one that can be simply supported
with a clear_cache() method on QuerySet or whatever.
> And I don't grant that
> Article.objects acting as a set is _completely_ non-surprising.
>
> Consider Python dictionaries. If you have a dictionary, you call:
>
> for key, value in my_dict.items(): ...
>
> _not_
>
> for key, value in my_dict.items:...
>
> Why? Because my_dict.items looks like it's an attribute of my_dict -
> which it isn't - its an iterator: a stateful entity unto itself, so it
> is produced using a factory method on the dictionary.
This seems to have almost no relevance, and is incorrect, to boot. The
thing returned is a list, not an iterator. This is a historical accident
- if dictionaries were being defined now, it would be a set IMO. And I
think you will find that properties and descriptors were not a feature
of python when the interface of dictionaries was defined. I really see
no reason why the second form of items wouldn't be used nowadays.
If it were an iterator - you would still have no point. The cache makes
no difference as to the *result* of operations, only to the *speed*.
This is entirely different to the state changes in an iterator, which
affect the result of the operations on the iterator.
> Queries are even more stateful than iterators, because they can be
> cached and reused, so they have the need to be both reset and
> re-executed over the database. How does this cache/reuse match the use
> expectations of a set as a standalone object?
Cache is generally not considered to be state in the sense that you are
using it. Eg in systems with const-ness present, caches are generally
marked mutable. They *do not change the results of operations, only the
efficiency*.
Obviously there is a caveat in here about concurrent changes and cache
consistency. I think most people are able to handle that. ( In the vast
majority of cases by ignoring it because their whole business
transaction is contained in one database transaction.)
Robert
Russell Keith-Magee
>
> Russell Keith-Magee wrote:
> > On 1/27/06, Robert Wittams <rob...@wittams.com> wrote:
> > It wasn't an attempt to be confusing.
>
> The intent does not change the effect.
Steady on soldier. Just because I disagree with you doesn't
_necessarily_ mean I'm an idiot. :-)
> > and how do you reset it?
>
> I think this is a legitimate need, and one that can be simply supported
> with a clear_cache() method on QuerySet or whatever.
... which is what I proposed in the first place.
I'm starting to get the feeling that we're talking at crossed purposes
here. And I think I know why.
The problem is I've been thinking of Article.objects() where the
return value is an object representing an instance of a _query_, which
can be modified, executed, reset, cached, or whatever. You, on the
other hand, are looking at Article.objects being as a higher order
representation of the underlying _set_.
In your context, I'll agree that what I've been proposing doesn't make
much sense, because there _is_ only one higher order representation of
the underlying data. If you need to filter, you have a different set.
Your version doesn't need Article.objects.all(), because
Article.objects _is_ all. The only problem here is concurrency, which,
as you rightly pointed out, is pretty close to a non-issue.
From my context, Article.objects() constructs a basic query that knows
how to return all object instances of type Article, which can then be
filtered to produce more complex query objects. The idea that there is
a single basic query which everyone should use doesn't really make
much sense - each query you want to make is unique, so it should have
its own cache/state/etc.
Half the confusion lies in the fact that my approach _could_ use your
syntax, and vice versa, but on both counts, it wouldn't make much
sense to do so, because of the way each solution represents the
underlying problem.
The rest of the confusion (at least on my part) stems is that there
are a lot of references in this thread and the wiki to Article.objects
being the Manager, with all()/filter() etc being used to get sets from
the Manager; as a result, my mental picture of the Manager isn't one
of a higher order set, it is one of managing SQL composition and a
connection to a database. If you need to call objects.all(), then
'objects' by itself doesn't really represent anything other than a
namespace for functionality.
Does this sound like I have a better understanding of your position
now? Does it clarify mine at all (not that you necessarily agree, but
at least see where I'm coming from)?
> > Consider Python dictionaries. If you have a dictionary, you call:
>
> This seems to have almost no relevance, and is incorrect, to boot.
OK - the list/iterator confusion was a bit of a brainfart on my part,
brought on by a long day and a late night in my part of the world.
However, I would still argue that dicts _do_ use the items() notation,
and consistency with the mother language is as least as important as
exploiting the niftyness that is properties.
Russ Magee %-)
Robert Wittams
>>
>>I think this is a legitimate need, and one that can be simply supported
>>with a clear_cache() method on QuerySet or whatever.
> The problem is I've been thinking of Article.objects() where the
> return value is an object representing an instance of a _query_, which
> can be modified, executed, reset, cached, or whatever. You, on the
> other hand, are looking at Article.objects being as a higher order
> representation of the underlying _set_.
This does accurately summarise the original idea, yes. I really don't
see why people want to make .objects callable. It just seems to be worse
in pretty much every situation.
> In your context, I'll agree that what I've been proposing doesn't make
> much sense, because there _is_ only one higher order representation of
> the underlying data. If you need to filter, you have a different set.
> Your version doesn't need Article.objects.all(), because
> Article.objects _is_ all. The only problem here is concurrency, which,
> as you rightly pointed out, is pretty close to a non-issue.
>
> From my context, Article.objects() constructs a basic query that knows
> how to return all object instances of type Article, which can then be
> filtered to produce more complex query objects. The idea that there is
> a single basic query which everyone should use doesn't really make
> much sense - each query you want to make is unique, so it should have
> its own cache/state/etc
The only ones which will be shared are the ones which are the same
object... which necessarily implies that the query/result set will be
the same. The query specification in any particular "QuerySet" is
immutable. The cache is just an optimisation. Make sense?
> Half the confusion lies in the fact that my approach _could_ use your
> syntax, and vice versa, but on both counts, it wouldn't make much
> sense to do so, because of the way each solution represents the
> underlying problem.
>
> The rest of the confusion (at least on my part) stems is that there
> are a lot of references in this thread and the wiki to Article.objects
> being the Manager, with all()/filter() etc being used to get sets from
> the Manager; as a result, my mental picture of the Manager isn't one
> of a higher order set, it is one of managing SQL composition and a
> connection to a database. If you need to call objects.all(), then
> 'objects' by itself doesn't really represent anything other than a
> namespace for functionality.
>
> Does this sound like I have a better understanding of your position
> now? Does it clarify mine at all (not that you necessarily agree, but
> at least see where I'm coming from)?
The whole point of the descriptor fields proposal was to make django orm
code look a lot more like standard python code, rather than java code (
getter methods abounding). I have no idea where the .all() thing came
from, and have always argued against it... to me, this seems like having
to do [1,2,3].all() to construct a list - it puts up an artificial
barrier between the collection and its contents. I've got no idea why
people want to do that - I'm assuming it is because they are hung up on
the idea of knowing exactly when database calls happen or something.
Sorry if I've been abrasive, its a mixture of annoyance at not having
the time to do this myself, and seeing the same arguments as happened on
IRC and this list a while ago repeated. This is really my fault for not
implementing it all earlier ;-(
Anyway, thanks to anyone who is going to implement this stuff, it should
make django programming a lot more natural imo.
Maniac
>From my context, Article.objects() constructs a basic query that knows
>how to return all object instances of type Article, which can then be
>filtered to produce more complex query objects. The idea that there is
>a single basic query which everyone should use doesn't really make
>much sense
>
Why not? Article.objects represent a concept of 'select * from table' -
a single immutable query which serves as a basis for appending to it
filters, orders etc. There is no need (semantically) to construct such
basic query each time since all later alterations don't actually change
the query but only use it as a prototype and produce new instances
themselves.
Article.objects() would be appropriate for this case:
q = Article.objects()
q.filter(...)
q.order(...)
for row in q:
...
But somewhere near we've already agreed that methods producing new
instances would be more useful.
>The rest of the confusion (at least on my part) stems is that there
>are a lot of references in this thread and the wiki to Article.objects
>being the Manager, with all()/filter() etc being used to get sets from
>the Manager;
>
There is a proposition also somewhere near this discussion to let
Manager be also a basic Query. Which turns out to cause a certain confusion.
Adrian Holovaty
http://code.djangoproject.com/wiki/DescriptorFields ? I got lost. :)
Maniac
>So what did we decide to change from the current proposal at
>http://code.djangoproject.com/wiki/DescriptorFields ? I got lost. :)
>
>
The best I can conclude is that the only difference with this page is
that people don't like the ".all()" thing and want Manager to behave
also as Query.
But I could be lost as much as anyone else :-)
Joseph Kocherhans
>
> Adrian Holovaty wrote:
>
> >So what did we decide to change from the current proposal at
> >http://code.djangoproject.com/wiki/DescriptorFields ? I got lost. :)
> >
> >
> The best I can conclude is that the only difference with this page is
> that people don't like the ".all()" thing and want Manager to behave
> also as Query.
I think that's the main point of disagreement. Does this capture it all?
1. Model.objects vs. Model.objects.all()
(and also instance.sites vs. instance.sites.all()?)
2. len(q) vs. q.count()
3. related_name for ForeignKey and ManyToManyField
related_name defaults to OBJECT_set if not specified
Joseph
Jacob Kaplan-Moss
> I think that's the main point of disagreement. Does this capture it
> all?
>
> 1. Model.objects vs. Model.objects.all()
> (and also instance.sites vs. instance.sites.all()?)
>
> 2. len(q) vs. q.count()
>
> 3. related_name for ForeignKey and ManyToManyField
> related_name defaults to OBJECT_set if not specified
My votes:
1. Model.objects
2. instance.sites
3. q.count()
4. related_name: yes
Jacob
Adrian Holovaty
> > (and also instance.sites vs. instance.sites.all()?)
> >
> > 2. len(q) vs. q.count()
> >
> > 3. related_name for ForeignKey and ManyToManyField
> > related_name defaults to OBJECT_set if not specified
>
> My votes:
>
> 1. Model.objects
> 2. instance.sites
> 3. q.count()
> 4. related_name: yes
My votes are exactly the same. Onward!
Luke Plant
> 1. Model.objects vs. Model.objects.all()
> (and also instance.sites vs. instance.sites.all()?)
+1 on Model.objects and instance.sites
> 2. len(q) vs. q.count()
Actually, can we get away with not having len(q)? If you try to use an
iterator in a template with {{ for foo in iterator_obj }} you get:
Exception Type: TypeError
Exception Value: len() of unsized object
You can kind of fix this, but what about using 'reversed'? That
definitely needs to do len(). (You could possibly special case Query
instances in the template code, by adding a .reversed() method which
inverts the order_by tuple ... but that's obviously evil)
In light of this, my earlier proposal was wrong. Instead it should be:
len(q) - retrieves the list, and does len() on it
bool(q) - retrieves the list, and does bool(len()) on it
q.count() - does a select count(*)
That actually makes more sense - the special method (count) does
something special, the Python builtins just force the data to be
fetched.
> 3. related_name for ForeignKey and ManyToManyField
> related_name defaults to OBJECT_set if not specified
+1
Luke
Brant Harris
I figured this out months ago:
http://code.djangoproject.com/ticket/980, we didn't have the concept
of the manager back then, of course, but the basic principles are
there, (query == filter).
If you cache the count, len() and bool() can be called over and over
for all you care.
Model.objects can't be a query, it's the manager. Sure it sounds like
all the objects, but that's what you have all() for. Although it
appears elegant at first, I think it will cause more problems that it
is worth.
The query object needs to query (or return a cache) on:
__iter__, __len__, __getitem__, __contains__, __bool__, and __str__
(__repr__ optionally, although I thought it would be more usefull to
return the created SQL, though)
It should cache the results always, but then define a "fetch()"
function, which immediately resends and returns the query, and
probably a "clear_cache()" function as RJ suggested.
_set, I waiver on. It's certainly scientifically correct. I remember
though when RJ first suggested it, my immediate reaction was not the
noun 'set', but the verb 'to set', so I think it could cause some
confusion for new users. In the end I am unsure. What I am sure
about is that each Manager should have a get_related_manager(field).
This would be a factory function, that assigns object_set, so you
could redo it to do objects, if you wanted.
Reporter.objects.order_by('-lname', 'fname') is incorrect as far as I
can see. The manager cannot hold this state info. This is more
correct:
q = Reporter.objects.filter(fname='John')
q.order_by('-lname', 'fname')
or:
print Reporter.objects.filter(fname='John').order_by('-lname', 'fname')
Thanks
Robert Wittams
> On Friday 27 January 2006 22:25, Joseph Kocherhans wrote:
>
>
>>1. Model.objects vs. Model.objects.all()
>> (and also instance.sites vs. instance.sites.all()?)
>
>
> +1 on Model.objects and instance.sites
>>2. len(q) vs. q.count()
>
>
> Actually, can we get away with not having len(q)? If you try to use an
> iterator in a template with {{ for foo in iterator_obj }} you get:
This bit me before with a generator - I ended up just doing
list(func()), but I think for should be fixed to work with iterators not
supporting len.
> Exception Type: TypeError
> Exception Value: len() of unsized object
>
> You can kind of fix this, but what about using 'reversed'? That
> definitely needs to do len(). (You could possibly special case Query
> instances in the template code, by adding a .reversed() method which
> inverts the order_by tuple ... but that's obviously evil)
> In light of this, my earlier proposal was wrong. Instead it should be:
> len(q) - retrieves the list, and does len() on it
> bool(q) - retrieves the list, and does bool(len()) on it
> q.count() - does a select count(*)
>
> That actually makes more sense - the special method (count) does
> something special, the Python builtins just force the data to be
> fetched.
I think this makes sense...
>>3. related_name for ForeignKey and ManyToManyField
>> related_name defaults to OBJECT_set if not specified
> +1
+1
Russell Keith-Magee
> The only ones which will be shared are the ones which are the same
> object... which necessarily implies that the query/result set will be
> the same. The query specification in any particular "QuerySet" is
> immutable. The cache is just an optimisation. Make sense?
Yup. On the same page now. You're, right, I'm wrong, you can stop
beating me now :-)
> The whole point of the descriptor fields proposal was to make django orm
> code look a lot more like standard python code, rather than java code (
> getter methods abounding). I have no idea where the .all() thing came
> from, and have always argued against it...
Absolutely. This is what was throwing me off.
> Sorry if I've been abrasive, its a mixture of annoyance at not having
> the time to do this myself, and seeing the same arguments as happened on
> IRC and this list a while ago repeated.
Ah - if it happened on IRC, I don't know about it. One of the side
effects of living in GMT+8 is that I don't have much of an window of
opportunity for live chat :-)
Sorry for making you repeat yourself. If it makes you feel better,
think of the good that has come of it :-)
Russ Magee %-)
Robert Wittams
> Reporter.objects.order_by('-lname', 'fname') is incorrect as far as I
> can see. The manager cannot hold this state info.
You appear to have misinterpreted the proposal then. There is a limit to
how many times the same argument can be repeated... please, reread the
thread. I really have no idea what the big brain block on this is.
Hint: Every function that would affect the query sql returns a new query
set. The query specification of each one is immutable.
Robert
Russell Keith-Magee
> > My votes:
> >
> > 1. Model.objects
> > 2. instance.sites
> > 3. q.count()
> > 4. related_name: yes
+1 for 1,2,4.
On 3 I'm not completely convinced. I like the elegance/language symmetry of
len(Model.objects). I can see two possible objections with allowing it:
- Firstly, how does __len__ fit in with query execution? I would argue that
len(Model.objects) should effectively return len(list(Model.objects)).
This would mean that the cache would be fully populated as soon as you
call len(). This is less efficient if you don't intend to use the
contents of the object set, but 1) this strikes me as the edge case,
and 2) we could retain objects.count() call as a non-caching size
call.
- Secondly, the template language doesn't lend itself to making the
len(Model.objects) call. One solution here - modify the template
lookup order to add global functions taking a single parameter as one
of the lookup options: i.e., {{ object.attr }} will be evaluated as:
object[attr]
object.attr
object.attr()
attr(object)
I'm unsure of the exact order of precedence, though. Last resort seems
right to me.
The counterobjection to the template change is that as a result of the
choice of python keyword, {{ Model.objects.len }} isn't that intuitive
to template writers. Also, it could start to encourage people to start
using global functions as filter-like objects, which we probably don't
want to do.
Russ Magee %-)
Luke Plant
> - Secondly, the template language doesn't lend itself to making the
> len(Model.objects) call.
{{ objects|length }} works fine.
If you wanted it in an 'if' tag (ie. if length = some value), then you
can't do the above, but you can't do it now either. What you can do
now is this:
{% if objects %}
[ ... print out objects... ]
{% else %}
Sorry, no objects today.
{% endif %}
With my proposal in the other e-mail, this would work exactly as before
- bool(objects) would fetch the data, and so you would do just the one
query as before.
Luke
--
"I married Miss Right, I just didn't know her first name was 'Always'"
Russell Keith-Magee
> On Saturday 28 January 2006 00:33, Russell Keith-Magee wrote:
>
> > - Secondly, the template language doesn't lend itself to making the
> > len(Model.objects) call.
>
> {{ objects|length }} works fine.
... as Russell manages to completely forget the obvious. You're
absolutely right. Scratch the template change.
> With my proposal in the other e-mail, this would work exactly as before
> - bool(objects) would fetch the data, and so you would do just the one
> query as before.
+1 to that proposal (__len__ and count(), with __len__ populating cache)
Russ Magee %-)
Carlo C8E Miron
2006/1/26, Adrian Holovaty <holo...@gmail.com>:
> I've written up my latest proposal here:
> http://code.djangoproject.com/wiki/DescriptorFields
Sorry if I'm late again, but I'm wondering...
why does QuerySet objects doesn't have
the methods all(), dates(), in_bulk() and values()
like Manager objects?
HAND,
(c)
--
Carlo C8E Miron, ICQ #26429731
--
Disclaimer:
If I receive a message from you, you are agreeing that:
1. I am by definition, "the intended recipient".
2. All information in the email is mine to do with as I see fit and
make such financial profit, political mileage, or good joke as it
lends itself to. In particular, I may quote it on USENET or the WWW.
3. I may take the contents as representing the views of your company.
4. This overrides any disclaimer or statement of confidentiality that
may be included on your message.