Re: Feature proposal: Q.push(...) and QuerySet.filter(relfield=Q(...))

2011-10-04 Thread Johannes Dollinger
Am 03.10.2011 um 20:01 schrieb Javier Guerra Giraldez:

> On Fri, Sep 30, 2011 at 4:37 PM, Johannes Dollinger
> <emulb...@googlemail.com> wrote:
>> The aim of this proposal is to reuse Q objects for models that are related 
>> through FK or M2M fields.
> 
> i really want to have this feature!  so i went and did a quick
> implementation and created ticket #16979[1]
> 
> [1]:https://code.djangoproject.com/ticket/16979
> 
> really untested, but a variation that doesn't modify core seems to
> work well (but the API is far too ugly to share)

Thanks for starting this!
Why did you chose a staticmethod instead of an instancemethod?
While I'm not sold on the method name (perhaps prefix() or shift() or even 
__rshift__() would be clearer), I believe the ability to modify Q objects 
outside of filter()-Expressions is important.
Just like &, |, and ~, adding a prefix is an operation on Q objects. And the 
API should provide a way to manipulate them fully outside of QuerySets.

As an afterthought: The cleanest solution would probably be an implementation 
in Q.__init__.
The .push() method would then be unnecessary, as you could write Q(foo=Q(...)) 
instead of Q(..).push('foo').
And .filter()/.exclude() would just work, as they simply pass their arguments 
through to Q.__init__.

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Feature proposal: Q.push(...) and QuerySet.filter(relfield=Q(...))

2011-09-30 Thread Johannes Dollinger
The aim of this proposal is to reuse Q objects for models that are related 
through FK or M2M fields.
A simplified example would be a Q object like

>>> is_blue = Q(blue=True)
>>> Thing.objects.filter(is_blue)

that should be reused to filter the owners of things:

>>> User.objects.filter(things__blue=True).

Currently, the only way to reuse the Q object would be subquery:

>>> User.objects.filter(things__in=Thing.objects.filter(is_blue)).

I therefore propose the following API:

>>> Q(lookup=value).push('relfield')

which would be equivalent to

>>> Q(relfield__lookup=value)

and

>>> qs.filter(relfield=Q(lookup=value))

which would be equivalent to

>>> qs.filter(Q(lookup=value).push('relfield')).

We then could write

>>> User.objects.filter(things=is_blue)

which is shorter and produces more natural SQL than the subquery solution.

Thoughs? Suggestions?

__
Johannes


-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: prefetch_related - new feature suggestion

2011-09-27 Thread Johannes Dollinger

Am 27.09.2011 um 05:18 schrieb Luke Plant:

> On 27/09/11 03:23, Alex Gaynor wrote:
> 
>> I'm not a fan of this, for a few reasons, firstly: because it feels
>> wrong for a QuerySet to execute multiple queries.  This isn't a deal
>> breaker, just something that struck my conceptually initially.  Second I
>> disagree that it's difficult to do outside of core, it's not as easy as
>> it ought to be, but it's very possible (citation: I've done it :)).
> 
> Would you like to share your solution? I found it pretty difficult to
> come up with anything that:
> 
> 1) could be done on a per-query basis and
> 2) didn't require changes to the code that would use the QuerySet
> objects i.e. fully API compatible.

I don't believe (2) is an important requirement. It may even be 
counter-productive.
For me, the ideal solution would allow to filter or annotate the prefetched 
QuerySet – and thus, obj.relmanager.all() would be a bad API to access the 
prefetched objects.
I'd like to propose something that works more like annotations:

>>> 
Foo.objects.prefetch_related(related_list=Foo.related_objects.filter(x__gt=42))

This would require related object descriptors to return something like a 
ParametrizedQuerySet object when accessed via the model class, e.g.:

>>> Foo.related_set.filter(x__gt=42).apply(*foo_objects)
# equivalent to: Related.objects.filter(foo__in= 
foo_objects).filter(x__gt=42)

Or, simpler, but less generic:

>>> Foo.related_set.filter(x__gt=42)
'foo', Related.objects.filter(x__gt=42)

This syntax would also make it somewhat obvious that you're going to use more 
than one db query.
Finally, and to support Alex point, if you drop your second requirement, it's 
well possible to implement this (modulo my proposed API) with the public 
QuerySet API + some introspection.

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: ManyRelatedManager with explicit intermediary model

2011-09-20 Thread Johannes Dollinger

Am 20.09.2011 um 15:57 schrieb Łukasz Rekucki:

> On 20 September 2011 15:52, Roald de Vries  wrote:
>> Hi all,
>> 
>> Is there a fundamental reason that I'm missing (other than "nobody's taken
>> the trouble of writing it") that I can't do the following? If there isn't
>> I'll create a ticket for it.
>> 
>>class R(Model):
>>user = ForeignKey(User)
>>my_model = ForeignKey('MyModel')
>>comment = CharField(max_length=100, blank=True)
>> 
>>class MyModel(Model):
>>users = ManyToManyField(User, through=R, null=True)
>> 
>>m = MyModel.objects.create()
>>u = User.objects.create_user('roald', 'downa...@gmail.com', 'password')
>> 
>> 
>># these things I can't do:
>>m.users.add(u)
>>m.users.add(u, comment='Blablabla')
>> 
>> Cheers, Roald
>> 
> 
> I'm 100% sure there's *at least one* ticket for this. You just need to
> search for it and you'll probably find the discussion of this too.

#9475 [1] is the ticket for the default value case. Russell's comments indicate 
that the other case (providing extra attributes for the intermediary model) may 
also be in scope for this ticket.

[1] https://code.djangoproject.com/ticket/9475

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: math tag

2011-05-03 Thread Johannes Dollinger

Am 03.05.2011 um 18:27 schrieb Phui Hock:

> Or, in a sane world:
> 
> if x = {{ x }}, y = {{ y }},  {{ x }} + {{ y }} = {{ x_plus_y_res }}
> if x = {{ x }}, y = {{ y }}, {{ x }} * {{ y }} = {{ x_star_y_res }}
> 
> and so on.
>  
> While it is a common consensus that logic should never be in the template, 
> the "solution" on the other hand is inconvenient at best. 
> 
> I must admit the examples are badly written and do not reflect the actual 
> problem. The point here though, is that a general purpose math tag should be 
> available by default. *The* math tag may be too flexible for its own good. If 
> we have a dumber solution, I'm fine with that so long it supports the 
> simplest cases, and that would be + - * / % min max and ().

FWIW, here are some concrete use-cases for math in templates:

* __mul__: Display a value of 0.42 as 42%.
* __neg__/__abs__: Display the absolute value and signum of a number separately.
* log: Display a number on a logarithmic scale.
* floor/ceil: Display floored or ceiled values.

I'd still be -1 regarding a {%math%} tag. The cleanest solution would be proper 
support for common operators mixed with a set of default filters for primitive 
functions (e.g log, floor, ceil, abs).

__
Johannes


-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: ModelForm validation - a better way?

2011-04-29 Thread Johannes Dollinger

Am 29.04.2011 um 04:13 schrieb Carl Meyer:

> Hi all,
> 
> We have a number of tickets open (at least #12028, #13249, #13091,
> #15326, and #15860 -- #13091 is the active one) reporting problems with
> unique_together constraints in our attempts to validate arbitrary
> partial models, when validating a ModelForm with some fields excluded.
> 
> Eduardo Gutierrez and I have spent some time looking at this problem
> recently, and my main feeling is that validation of arbitrarily partial
> models in the face of unique_together constraints has no reliably right
> answer, and we'd do better to move away from it altogether.
> 
> Fortunately, I think we can do better. The reason we have to validate
> partial models is because of this idiom:
> 
> if form.is_valid():
>obj = form.save(commit=False)
>obj.user = request.user # for instance
>obj.save()
> 
> But there is no reason those tweaks to the model have to happen after
> form validation. If we encouraged an idiom where the tweaks happen
> before form validation, we could just run full model validation and
> avoid all the error-prone complexity of validating partial models.
> 
> Alex and I talked over some possibilities for a context manager
> available from a new method on ModelForms, that you'd use like this
> (idea originally from Łukasz Rekucki [1], somewhat modified):
> 
>def my_view(request):
>if request.method == "POST":
>form = MyModelForm(request.POST)
>with form.finish() as obj:
>obj.user = request.user
>if form.is_valid():
>return redirect(obj)
>else:
>form = MyForm()
>return render(request, "foo.html", {"form": form})
> 
> form.finish() returns a context manager which returns form.instance from
> its __enter__ method, as "obj" here, allowing the user to do any
> tweaking they like within the body of the context manager. On exit, the
> context manager performs _full_ model validation. Any validation errors
> are saved to the form, as usual. If validation succeeds, the model
> instance is saved.
> 
> The following check to form.is_valid(), then, is just for purposes of
> managing view logic (redirect, or redisplay form?). Actual validation
> has already happened, so it would just be checking self._errors (this
> isn't a change, that's already how .is_valid() works).
> 
> For backwards compatibility, there would be no change to the existing
> behavior if you don't use the new .finish() context manager -
> form.is_valid() would trigger possibly-partial validation just as it
> does now, and do the best it can. But the admin could be converted to
> use the new idiom (with a new ModelAdmin method that would be called
> from within the context manager to allow model-tweaking before
> validation), which would solve the admin-related bugs. And the
> documentation could recommend the new idiom over the old, particularly
> for incomplete modelforms with unique_together constraints.
> 
> Open questions:
> 
> 1. What if you don't need to tweak the instance, but you do want to
> trigger the new full validation behavior (i.e. your form excludes some
> fields, but you initially passed an instance to the form that you're
> confident has the excluded fields already set correctly - this would be
> the case for e.g. the parent FK in admin inlines)? Perhaps form.finish()
> could take an argument that determines whether it returns the context
> manager or just returns form.instance directly?
> 
> 2. Is it going to be too weird for people to adjust to the idea that
> they get their model instance out of the form before they (explicitly)
> call form.is_valid()?
> 
> Other issues we've overlooked, or ways this could be improved? Use cases
> this doesn't handle?

Here's my take on the API:

def my_view(request):
form = MyModelForm(request.POST or None)
try:
with form.finish() as obj:
obj.user = request.user
return redirect(obj)
except ValidationError:
return render(request, "foo.html", {"form": form})

The context manager would validate the form on __enter__  (only included 
fields) and raise ValidationError if it does not validate. On __exit__, it 
would raise ValidationError if model validation fails. This let's you defer 
expensive computations/queries until after a sanity check. Optionally, finish() 
might take a flag to defer all validation until __exit__, which may be required 
if you want to display all validation erros at once.
As an added benefit, this makes dealing with multiple forms and formset in a 
single view straight forward, as you can simply add more `with` blocks inside 
the try block, and validation across forms can simple raise a ValidationError 
on its own.

__
Johannes


-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email 

Re: #14891, a.k.a. "related managers don't work how we claim they do"

2011-04-18 Thread Johannes Dollinger

Am 17.04.2011 um 01:30 schrieb Carl Meyer:

> So - do we (A) fix the behavior to match our documented semantics, note
> it in the release notes, and hope for the best? Or (B) bow to
> backwards-compatibility and change the documentation to match the actual
> behavior? Or (C) find some middle ground (a deprecation path for the
> current behavior)?

I'd vote for (C). 
Deprecate `use_for_related_fields` and always use the default manager for 
related managers. Then add the possibility to specify custom mangers for 
individual relations:

 ForeignKey(Foo, related_manager=RSpecialManager)
 ManyToManyField(Foo, manager=SpecialManger, related_manager= 
RSpecialManager)

More fine grained control would especially be useful for subclasses of 
ForeignKey and ManyToManyField fields.
It comes at the expense of verbosity, but it appears to be a rarely used 
feature (given that the bug was discovered only now). And thus, explicitness 
might actually be a good idea.

And it would be a step towards discouraging use of multiple 
managers.

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: [GSoC] Composite fields: proposal, part 1

2011-04-06 Thread Johannes Dollinger

Am 06.04.2011 um 23:29 schrieb Michal Petrucha:

> On Wed, Apr 06, 2011 at 06:22:33AM +0200, Johannes Dollinger wrote:
>> 
>> Am 06.04.2011 um 02:45 schrieb Michal Petrucha:
>> 
>> [snip]
>> 
>>> unique and db_index
>>> ~~~
>>> Implementing these will require some modifications in the backend code.
>>> The table creation code will have to handle virtual fields as well as
>>> local fields in the table creation and index creation routines
>>> respectively.
>>> 
>>> When the code handling CompositeField.unique is finished, the
>>> models.options.Options class will have to be modified to create a unique
>>> CompositeField for each tuple in the Meta.unique_together attribute. The
>>> code handling unique checks in models.Model will also have to be updated
>>> to reflect the change.
>> 
>> Instead of auto-creating CompositeFields for each composite index
>> you could just append to _meta.unique_together in CompositeFields.
>> This prevents cluttering the set of field for introspection. Or is
>> there a benefit in using full CompositeFields for each index?
> 
> I proposed this change for the sake of API uniformity. I mean,
> currently there is one way to specify the unique constraint for single
> fields, a similar way to create an index and also a very similar way
> to specify the primary key. But if you want to specify a unique
> constraint on several fields, you have to use a completely different
> mechanism and an analogy for composite indexes is missing altogether.
> 
> Since I'm proposing an analogous API to specify a primary key
> consisting of several fields, and since it can also be extended to
> specify indexes the same way without much effort, it seems kind of
> "right" to me to unify the API. (I'm talking only about the public API
> here.)
> 
> I'll just note that this was not my idea, I just included it in my
> proposal when I was reading that thread because I consider it a good
> idea. See [47].

The only downside is that you'll have to pick a name for the index – even if 
you don't really care (that's historically been a problem with `related_name`). 
But anyway, since Meta.unique_together probably cannot be deprecated any time 
soon, that's just a -0 from me.

> As for the set of fields, I don't think having a few more fields in
> _meta.virtual_fields would hurt that much. It's not like every model
> is going to have tens of surpluss CompositeFields...
> 
>>> Retrieval and assignment
>>> 
>>> 
>>> Jacob has actually already provided a skeleton of the code that takes care
>>> of this as seen in [1]. I'll only summarize the behaviour in a brief
>>> example of my own.
>>> 
>>>   class SomeModel(models.Model):
>>>   first_field = models.IntegerField()
>>>   second_field = models.CharField(max_length=100)
>>>   composite = models.CompositeField(first_field, second_field)
>>> 
>>>>>> instance = new SomeModel(first_field=47, second_field="some string")
>>>>>> instance.composite
>>>   SomeModel_composite(first_field=47, second_field='some string')
>>>>>> instance.composite = (74, "other string")
>>>>>> instance.first_field, instance.second_field
>>>   (74, 'other string')
>> 
>> Just a small bikeshed: CompositeField should take the fields as a
>> tuple instead of *varargs.
> 
> Both solutions look equally "good" to me, even the difference in the
> implementation is just a single asterisk. (-; If it were up to me, I'd
> choose the *varargs version only because it would save the user a pair
> of parantheses.

You get to paint it :)

>>> QuerySet filtering
>>> ~~
>>> 
>>> This is where the real fun begins.
>>> 
>>> The fundamental problem here is that Q objects which are used all over the
>>> code that handles filtering are designed to describe single field lookups.
>>> On the other hand, CompositeFields will require a way to describe several
>>> individual field lookups by a single expression.
>>> 
>>> Since the Q objects themselves have no idea about fields at all and the
>>> actual field resolution from the filter conditions happens deeper down the
>>> line, inside models.sql.query.Query, this is where we can handle the
>>> filters properly.
>>> 
>>> There is already some basic machinery inside Query.add_filter and
>>> Query.setup_joins that is in use by GenericRelations, this is
>>

Re: [GSoC] Composite fields: proposal, part 1

2011-04-05 Thread Johannes Dollinger

Am 06.04.2011 um 02:45 schrieb Michal Petrucha:

[snip]

> unique and db_index
> ~~~
> Implementing these will require some modifications in the backend code.
> The table creation code will have to handle virtual fields as well as
> local fields in the table creation and index creation routines
> respectively.
> 
> When the code handling CompositeField.unique is finished, the
> models.options.Options class will have to be modified to create a unique
> CompositeField for each tuple in the Meta.unique_together attribute. The
> code handling unique checks in models.Model will also have to be updated
> to reflect the change.

Instead of auto-creating CompositeFields for each composite index you could 
just append to _meta.unique_together in CompositeFields.
This prevents cluttering the set of field for introspection. Or is there a 
benefit in using full CompositeFields for each index?

> Retrieval and assignment
> 
> 
> Jacob has actually already provided a skeleton of the code that takes care
> of this as seen in [1]. I'll only summarize the behaviour in a brief
> example of my own.
> 
>class SomeModel(models.Model):
>first_field = models.IntegerField()
>second_field = models.CharField(max_length=100)
>composite = models.CompositeField(first_field, second_field)
> 
 instance = new SomeModel(first_field=47, second_field="some string")
 instance.composite
>SomeModel_composite(first_field=47, second_field='some string')
 instance.composite = (74, "other string")
 instance.first_field, instance.second_field
>(74, 'other string')

Just a small bikeshed: CompositeField should take the fields as a tuple instead 
of *varargs.

> QuerySet filtering
> ~~
> 
> This is where the real fun begins.
> 
> The fundamental problem here is that Q objects which are used all over the
> code that handles filtering are designed to describe single field lookups.
> On the other hand, CompositeFields will require a way to describe several
> individual field lookups by a single expression.
> 
> Since the Q objects themselves have no idea about fields at all and the
> actual field resolution from the filter conditions happens deeper down the
> line, inside models.sql.query.Query, this is where we can handle the
> filters properly.
> 
> There is already some basic machinery inside Query.add_filter and
> Query.setup_joins that is in use by GenericRelations, this is
> unfortunately not enough. The optional extra_filters field method will be
> of great use here, though it will have to be extended.
> 
> Currently the only parameters it gets are the list of joins the
> filter traverses, the position in the list and a negate parameter
> specifying whether the filter is negated. The GenericRelation instance can
> determine the value of the content type (which is what the extra_filters
> method is used for) easily based on the model it belongs to.
> 
> This is not the case for a CompositeField -- it doesn't have any idea
> about the values used in the query. Therefore a new parameter has to be
> added to the method so that the CompositeField can construct all the
> actual filters from the iterable containing the values.
> 
> Afterwards the handling inside Query is pretty straightforward. For
> CompositeFields (and virtual fields in general) there is no value to be
> used in the where node, the extra_filters are responsible for all
> filtering, but since the filter should apply to a single object even after
> join traversals, the aliases will be set up while handling the "root"
> filter and then reused for each one of the extra_filters.

As you noted, the problem with GFK, CompositeFields, and virtual fields in 
general is mostly that support for filtering (read: custom lookups) requires 
changes deep inside the ORM.
I have a half-baked proposal that would make such fields much simpler to 
implement but would require a large refactoring of the ORM. It might well be 
that it turns out to be impractical, impossible, or too big to be included in 
your summer of code project. That being said, here's the plan:

Lookups are handled by the field class. That involves moving significant bits 
of code from db.models.sql.query into the field classes. Ideally, Query knows 
anything about fieldtypes. It just delegates lookups (including related field 
lookups) to the field implementation via:

def add_lookup(self, query, alias, lookup_bits, value, **kwargs):
"""
- `query` is the db.sql.query.Query instance
- `alias` is the table alias of the "current" table; that is: 
the table which this lookup must be performed on
- `lookup_bits` is a sequence of lookups, e.g. 
"related_field__some_field__icontains".split("__")
- `value` is the filter argument
- `**kwargs`: It might be necessary to pass more information, 
but certainly not arbitrary data. I just 

Re: Single Table Inheritance

2011-03-29 Thread Johannes Dollinger

Am 29.03.2011 um 20:46 schrieb Shawn Milochik:

> They can create a custom manager on the abstract class that would
> return an iterable, perhaps using itertools.chain() of the querysets.
> 
> It depends on what they expect to do with the output of this custom
> manager, and they'd obviously lose the ability to treat this output as
> a queryset by using additional sorts & filters and such. But if the
> goal is to be able to get instances from all subclasses at once then
> this is a viable solution, FWIW.

FWIW, here is an implementation that does just that: 
https://github.com/emulbreh/shrubbery/blob/master/shrubbery/db/union.py

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Composite primary keys

2011-03-21 Thread Johannes Dollinger

Am 21.03.2011 um 08:33 schrieb Christophe Pettus:

> I'd like to make one more pitch for a slightly different implementation here. 
>  My concern with CompositeField isn't based on the fact that it doesn't map 
> one-to-one with a field in the table; it's that it doesn't have any of the 
> semantics that are associated with a field.  In particular, it can't be:
> 
> - Assigned to.
> - Iterated over.
> - Or even have a value.

You would be able to use composite fields normally (as in "normal django 
field"):

>>> foo = Foo.objects.create(coords=(0, 0))
>>> foo.coords
(0, 0)
>>> foo.coords = (4, 2)
>>> foo.coords.x # == foo.x == foo.coords[0]
4 

Sidenote:: Subclassing the default implementation of composite field values 
should be easy:

>>> type(foo.coords)

>>> foo.coords.length
4.4721359549995796
>>> foo.coords += foo.velocity

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Composite primary keys

2011-03-16 Thread Johannes Dollinger
I would be nice if support for composite primary keys would be implemented as a 
special case of general composite fields. There would be no need for new Meta 
options:

class Foo(Model):
x = models.FloatField()
y = models.FloatField()
coords = models.CompositeField((x, y), db_index=True)
a = models.ForeignKey(A)
b = models.ForeignKey(B)
pair = models.CompositeField((a, b), primary_key=True)

A CompositeField descriptor would then return a namedtuple of its values and 
would support queries:

filter(coords__x=42)
filter(coords=(1,2))

Adding the individual fields may be optional, e.g, 
CompositeField((FloatField(), FloatField()), db_index=True).

This has been proposed before: 
http://groups.google.com/group/django-developers/browse_thread/thread/32f861c8bd5366a5

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Feature proposal: escape hatch for colliding template syntax in django templates

2010-10-20 Thread Johannes Dollinger

Am 20.10.2010 um 10:40 schrieb Andrew Godwin:

> On 20/10/10 02:40, Stephen Kelly wrote:
>> Sorry. Sent too early. All thumbs today. Consider these examples:
>> 
>> {% verbatim "%} %}" %}
>> 
>> (That is, "%} %}" in a verbatim-no-end tag)
>> 
>> {% verbatim %} %} %} {% endverbatim %}
>> 
>> (That is, " %} %} " wrapped in verbatim tags)
>> 
>> The current lexer uses regexps to find tokens like that. It would need to be
>> completely rewritten/redesigned to handle these cases.
>> 
>> All the best,
>> 
>> Steve.
>> 
>>   
> 
> Are you sure? There's no nesting here, so I'm reasonably sure this could be a 
> regular language, though I don't want to sit down and prove that.

A verbatim tag would at least introduce ambiguity:

{% verbatim %}{% endverbatim %}{% verbatim %}{% endverbatim %}

This could be parsed as [Verbatim(""), Verbatim("")] or [Verbatim("{% 
endverbatim %}{% verbatim %}")].

For the inline case, e.g. {% verbatim "%}" %}, it might be cleaner to just 
allow template tag delimiters in strings inside var nodes -  the lexer would 
have to be modified anyway:

{{ '%}' }} or even {{ foo|prefix:'{%'|suffix:'%}' }}

__
Johannes



-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: combining querysets with isnull

2010-10-14 Thread Johannes Dollinger

Am 14.10.2010 um 23:07 schrieb Dan Watson:

> There seems to be some inconsistent behavior when combining querysets
> that use isnull on a ForeignKey. I'm not sure how to explain it well
> in plain english, so here's a boiled-down example:
> 
> # Models
> 
> class Item (models.Model):
>title = models.CharField( max_length=100 )
> 
> class PropertyValue (models.Model):
>label = models.CharField( max_length=100 )
> 
> class Property (models.Model):
>item = models.ForeignKey( Item, related_name='props' )
>key = models.CharField( max_length=100 )
>value = models.ForeignKey( PropertyValue, null=True )
> 
> # Example
> 
> item = Item.objects.create(title='Some Item')
> pv = PropertyValue.objects.create(label='Some Value')
> item.props.create(key='a', value=pv)
> item.props.create(key='b')
> q1 = Q(props__key='a', props__value=pv)
> q2 = Q(props__key='b', props__value__isnull=True)
> qs1 = Item.objects.filter(q1) & Item.objects.filter(q2)
> qs2 = Item.objects.filter(q2) & Item.objects.filter(q1)
> 
> I would have expected qs1 and qs2 to yield the same result, but they
> do not. They should both return the single item, but qs1 returns
> nothing. The SQL for qs1 looks like:
> 
> SELECT "app_item"."id", "app_item"."title" FROM "app_item"
> INNER JOIN "app_property" ON ("app_item"."id" =
> "app_property"."item_id")
> LEFT OUTER JOIN "app_property" T4 ON ("app_item"."id" = T4."item_id")
> LEFT OUTER JOIN "app_propertyvalue" T5 ON ("app_property"."value_id" =
> T5."id")
> WHERE (("app_property"."value_id" = 1  AND "app_property"."key" =
> 'a' ) AND (T5."id" IS NULL AND T4."key" = 'b' ))
> 
> The first app_property join corresponds to q1, and the second
> corresponds to q2. However, the app_propertyvalue join (corresponding
> to the isnull from q2) refers to app_property.value_id (i.e. q1)
> instead of T4.value_id (i.e. q2). I think fixing #10790 would fix
> this, since removing the join to app_propertyvalue and simply checking
> "T4.value_id IS NULL" works as expected, but I'm not sure if this is a
> separate problem worthy of another ticket or not. Also (less
> importantly), since both criteria (q1 and q2) are checking
> props__key='something', I would imagine both joins could be INNER
> JOINs, but the isnull seems to promote the second to a LEFT OUTER
> JOIN.
> 
> I guess I was just wondering if I should open a separate ticket for
> this, or is this simply a symptom of #10790? Or am I missing
> something?

Your problem looks like #11052 [1].

[1] http://code.djangoproject.com/ticket/11052

__
Johannes



-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: #6735 -- Class based generic views: call for comment

2010-10-01 Thread Johannes Dollinger

Am 01.10.2010 um 18:07 schrieb Luke Plant:

> On Fri, 2010-10-01 at 12:16 +0200, Johannes Dollinger wrote:
>> Am 01.10.2010 um 07:26 schrieb Russell Keith-Magee:
>>> I've just added a summary of the last thread on class-based views
>> [1].
>>> This summary isn't 100% complete -- any contributions from
>> interested
>>> parties are welcome. Try to keep opinions to a minimum; this page is
>>> about documenting the strengths and weaknesses of various
>> approaches,
>>> not about expressing opinions. In the same way that CSRF [2] and
>>> session-messages [3] have good wiki pages describing the design
>>> considerations, we need to be able to explain to future generations
>>> why class-based views are the way they are.
>> 
>> Could you (or anyone knowledgable) add a section, that explains why
>> each request should have its own view instance?
>> The thread-safety argument alone does not suffice: if all _request_
>> state would be stored on request instead of the view, you wouldn't
>> need new instances per request. You could also pass around state
>> explicitly - which admittedly gets messy quickly.
>> So is this purely about preventing users from shooting themselves in
>> the foot? (hyperbole: Will there be protection from globals in django
>> 1.4?)
> 
> It's not just about stopping users from shooting themselves in the foot,
> it's about helping them to do the right thing easily.  Without this kind
> of protection, it will be harder for *anyone*, including experienced
> developers who are aware of the problem, to do things correctly. It's
> like the autoescaping in templates - I *know* that I should use the
> 'escape' filter, but without autoescaping it is hard for anyone to do it
> right all the time.
> 
> One alternative that has been suggested is to pass state around
> explicitly, which is definitely best practice, but in many cases might
> not be possible (e.g. if you are working with someone else's base class,
> and in one overridden method you want to set up some objects which ought
> to be available to other methods). You could also attach stateful data
> to the request object, but attaching random bits of data to the request
> is surely just as ugly a solution as the self.copy() call, especially if
> you need to avoid names clashes with all the other attributes attached
> to request.
> 
> With regards to doing a shallow copy, it should be noted that at the
> point the copy is made, the only data attached to the object is data
> that has been attached in the constructor. It is of course possible that
> methods within the view might mutate such data, but it seems more likely
> that it will be used as immutable configuration data. 
> 
> However, I have already seen some example code that might fall foul of
> this problem - someone gave the example of storing a queryset on the
> object in the __init__. This will get implicitly mutated (the cache is
> filled) when it is used the first time. This could lead to frustrating
> problems that wouldn't be found in testing, and doing copy() on the
> instance won't help.
> 
> So, in light of the fact that developers *will* need to be aware of this
> issue, I'd like to tentatively suggest an explicit solution which is
> nonetheless easy to use and get right. We could have an explicit 'state'
> object that is thrown away for every new request, something like this:
> 
> class State(object):
>pass
> 
> class View(object):
>def __call__(self, request, *args, **kwargs):
>"""
>Main entry point for a request-response process.
>"""
>self.state = State()
>self.request = request
>self.args = args
>self.kwargs = kwargs
>resp = self.dispatch(request, *args, **kwargs)
># let any state get GC'd immediately:
>del self.state 
>del self.request
>del self.args
>del self.kwargs
>return resp
> 
> We document the issue, warn people not to store state on the instance
> itself, but tell them that if they must have stateful data, it should be
> attached to self.state instead of self, and they will be OK.  They might
> still be bitten if they put mutable configuration data into the __init__
> and it gets mutated, but we have made it easy to do the right thing -
> doing 'self.state.x' is only slightly harder than 'self.x'
> 
> Thoughts?


If you are willing to limit view state to a dedicated container, you might as 
well put in on request (e.g. request.view, request.state, request.context, ...) 
- and store args/kwargs in it as well.
There would be no need to copy or clear self. It could be ad

Re: #6735 -- Class based generic views: call for comment

2010-10-01 Thread Johannes Dollinger
Am 01.10.2010 um 07:26 schrieb Russell Keith-Magee:
> I've just added a summary of the last thread on class-based views [1].
> This summary isn't 100% complete -- any contributions from interested
> parties are welcome. Try to keep opinions to a minimum; this page is
> about documenting the strengths and weaknesses of various approaches,
> not about expressing opinions. In the same way that CSRF [2] and
> session-messages [3] have good wiki pages describing the design
> considerations, we need to be able to explain to future generations
> why class-based views are the way they are.

Could you (or anyone knowledgable) add a section, that explains why each 
request should have its own view instance?
The thread-safety argument alone does not suffice: if all _request_ state would 
be stored on request instead of the view, you wouldn't need new instances per 
request. You could also pass around state explicitly - which admittedly gets 
messy quickly.
So is this purely about preventing users from shooting themselves in the foot? 
(hyperbole: Will there be protection from globals in django 1.4?)

> Based on the discussion on the most recent thread [4], plus in-person
> discussions, I'm proposing that we move forward with Ben Firshman and
> David Larlet's "__call__ + copy()" approach. Yes, the copy() is a bit
> of a weird idiom, but it's not an idiom that users need to be
> particularly concerned with; the code "just works" in an unsurprising
> way.

I disagree. If you define a class-based view, create an instance (a singleton) 
and put it in your urlconf, you wouldn't (or shouldn't) expect it to magically 
reinstantiate itself everytime it's called. So at least the docs will have to 
point out the magic prominently.

Regardless of my resentments, thanks for finally creating an official 
foundation for class based views. 

__
Johannes

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Class based generic views in 1.3?

2010-06-17 Thread Johannes Dollinger

Am 17.06.2010 um 18:23 schrieb Ian Lewis:


The example he provided isn't terribly good but you don't need an view
instance per request (the example will work unless you start adding
stuff to self or overwriting self.qs etc). Only shared state is stored
at the class level and view customization is done at the class
instance level as well. That state shouldn't change.

Attaching all kinds of stuff to self tends to depend on the order of
the methods called and allows for some ugly coding styles. In that
case the slightly strange idea of merging the request and the view by
extending HttpRequest starts to make sense since otherwise you would
have some state on the request object (like the user etc. as it is
now) and some state on the view object.

That said, both extending HttpRequest and using __new__ seem like a
hacks to me. Personally the idea of a view being a callable class
instance or method is simple and flexable enough to handle any use
cases we are talking about. You just have to think of self as the
*view* rather than a thread safe request because that's what it is.

Having a view object per request makes no sense. You should have a
request object per request.  It is largely a result of wanting the
convenience of being able to write sloppy code that writes all kinds
of state to self rather than a technical necessity.

Ian


I'd like to support Ian and Patryk's position: class based views  
should not be instantiated for each request - for all the reasons Ian  
gave in the quoted mail.
If you want to avoid passing around (lots of) arguments between  
methods, you can always store request specific state on the request  
instance, which is a common pattern for middleware already.
To prevent the request's dict from becoming too cluttered, simply put  
the arguments into `request.view_context` (a dict or similar  
container, needs to be painted).


__
Johannes

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Abstract models and their managers

2010-03-04 Thread Johannes Dollinger

Hi Steve,

Multiple inheritance with abstract models works, and mostly did since  
the feature was added afaict. I use it regulary.
Just stay away from diamond inheritance and multi-multi-table  
inheritance.


Regarding your managers: couldn't you just use inheritance explicitly?

  class ManagerC(ManagerA, ManagerB): pass

  class ModelC(ModelA, ModelB):
objects = ManagerC()

__
Johannes

Am 03.03.2010 um 23:08 schrieb Stephen McDonald:


Hi Russel,

Thanks for your feedback. That's a really interesting position to
learn about with regard to multiple inheritance as I use it all the
time across basic abstract models without any issues.

The approach I was thinking of is very simplistic and possibly naive.
It appears as though I just need to modify
django.db.models.ModelBase.copy_managers to dynamically create the new
managers and assign those to the model class where overlapping
attribute names (eg objects) occur.

If anyone else has any further insight to whether this would work I'd
really appreciate it.

Steve




--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Call for feedback: django.utils.signed and signed cookies

2009-12-22 Thread Johannes Dollinger

Am 22.12.2009 um 08:20 schrieb Simon Willison:

> On Dec 22, 12:52 am, Johannes Dollinger
> <johannes.dollin...@einfallsreich.net> wrote:
>> I'd like some more kwargs to Signer and TimestampSigner. Mostly  
>> what's
>> inhttp://dpaste.com/136418/(except the `separator` kwarg, which was
>> a bad idea as it depends on encode()): Signer(serializer=...) and
>> TimestampSigner(ttl=...).
>
> The first few versions of the code had a bunch more stuff along the
> lines of that dpaste (which expires in 6 days, so here's a gist copy
> of it: http://gist.github.com/261572 )

Thanks.

> After struggling with it for quite a while, I decided that having a
> single class that handled signing, serialization and compression
> wasn't the right approach - it was doing far too much. Instead, I
> changed the design so the Signer's only responsibility was generating
> signatures, appending them to strings and verifying that they were
> correct. The encryption/serialization was extracted out and moved
> directly in to the signed.dumps() and signed.loads() functions.
>
> This solved a bunch of problems I was having with the code - too many
> subclasses, confusing amounts of multiple inheritance for mixing
> together different subclassed behaviours - and generally made
> everything feel a lot more cohesive.
>
> That's also why I added the BACKEND setting for setting a cookie
> signing backend - that way, users with specific ideas about how their
> signed cookies should work can write their own class that does pretty
> much anything they like.
>
> Is there any functionality in particular that you think should be
> resurrected from the more-complex-signing-classes? I'm very keen on
> keeping the serialization and compression stuff out of Signer.

I wanted to proposed to make dumps() and loads() methods on Signer  
again. Now they are bound to TimestampSigner. If they were defined as  
methods, you could reuse the functionality on subclasses. A mixin  
might be overengineering as you would never use it without Signer and  
it makes sense to have the feature available for all subclasses. Why  
do you want to keep it out?

Useful additions:
* If sign() and unsign() take a `sep` kwarg, Signer.__init__() should  
also take it. Better yet, only set it in __init__().
* TimestampSigner.__init__() needs a max_age/ttl kwarg.
__
Johannes

--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




Re: Call for feedback: django.utils.signed and signed cookies

2009-12-21 Thread Johannes Dollinger
There's a small bug in b64_decode(), the padding should be
r = len(s) % 4
pad = '=' * (r and 4 - r or 0)

I'd like some more kwargs to Signer and TimestampSigner. Mostly what's  
in http://dpaste.com/136418/ (except the `separator` kwarg, which was  
a bad idea as it depends on encode()): Signer(serializer=...) and  
TimestampSigner(ttl=...).
__
Johannes

Am 21.12.2009 um 12:43 schrieb Simon Willison:

> I've uploaded the patch for adding signing and signed cookies to
> Django:
>
> http://code.djangoproject.com/attachment/ticket/12417/ticket12417.diff
>
> You can also read the documentation directly on my GitHub branch:
>
> http://github.com/simonw/django/blob/signed/docs/topics/signing.txt
> http://github.com/simonw/django/blob/signed/docs/ref/request-response.txt#L224
> http://github.com/simonw/django/blob/signed/docs/ref/request-response.txt#L561
>
> Most of the code lives in django.utils.signed (the low-level signing
> API) but I've also added a get_signed_cookie() method to HttpRequest
> and a corresponding set_signed_cookie() method to HttpResponse:
>
> http://github.com/simonw/django/blob/signed/django/http/ 
> __init__.py#L84
> http://github.com/simonw/django/blob/signed/django/http/__init__.py#L406
> http://github.com/simonw/django/blob/signed/django/utils/signed.py
>
> The code has documentation and unit tests. The documentation isn't
> 100% complete - I need to improve the explanation of what signing is
> and why it is useful and document the new COOKIE_SIGNER_BACKEND
> setting which allows users to swap in their own cookie signing
> behaviour should they need to.
>
> Most importantly though, the implementation has not yet been peer
> reviewed by real cryptographers. With that in mind, would it be
> appropriate to check this in before the 1.2 freeze? We would certainly
> get the code reviewed before the final 1.2 release.
>
> Cheers,
>
> Simon
>
> --
>
> You received this message because you are subscribed to the Google  
> Groups "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com 
> .
> To unsubscribe from this group, send email to 
> django-developers+unsubscr...@googlegroups.com 
> .
> For more options, visit this group at 
> http://groups.google.com/group/django-developers?hl=en 
> .
>
>


--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




Re: Ticket #3349 patch review

2009-12-21 Thread Johannes Dollinger

Am 15.12.2009 um 19:57 schrieb Andrew Durdin:

> On Dec 14, 11:00 pm, ab  wrote:
>>> `wrap_and_raise()` will appear in the traceback, `raise
>>> wrap_exception(SomeException())` would be cleaner.
>>
>> I like that
>
> But you must use the three-argument `raise` statement to supply your
> own traceback in Python 2.x. You could dispense with the function
> entirely if you're happy to repeat `import sys; raise NewException(),
> None, sys.exc_info()[2]` instead--although then you lose some
> information, see below.

I pictured the solution to be closer to PEP 3109 / Java: The wrapped  
exception as an attribute on the new exception, with the traceback  
attached in `exp.__cause__.__traceback__`. That's where my  
`WrappingException(.., wrap=True)` came from.
I don't like the mangled tracebacks, but you're right - the reference  
cycle-free python 2.x solution probably is to use the tree-argument  
raise statement.

>>> Your patch seems to swallow the new exception's traceback now  
>>> instead
>>> of the causing traceback. That might be good in some situations, but
>>> generally both should be preserved.
>
> No; the only part of the traceback lost is the `raise` line within
> `wrap_and_raise`; the remainder of the traceback which would
> correspond to all but the `raise` line of the new exception's
> traceback is preserved. But if you weren't using the `wrap_and_raise`
> function, you would lose the line of the traceback where the new
> exception was raised. Put the following code in a python script and
> compare the tracebacks when it calls wrapped(), unwrapped(), or
> manually_wrapped():

My mistake, of course you're right.

__
Johannes

--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




Re: Ticket #3349 patch review

2009-12-14 Thread Johannes Dollinger

Am 14.12.2009 um 10:53 schrieb Andrew Durdin:

> I'm the author of the current patch; I'll just add a bit of
> background.
>
> On Dec 12, 10:18 pm, ab  wrote:
>>
>> 1. Scope -- the patch generalizes the issue and addresses it
>> throughout Django. Are people ok with that?
>
> Since the problem of raising new exceptions and losing the original
> one seemed systemic throughout much of Django, I thought it
> appropriate to make the patch address all known instances of this
> class of problem, and not just the one instance in the original
> complaint.

Yes, please.

>> 2. Design -- The wrap_and_raise function needs to be used not only at
>> the site of the problem, but also possibly at a deeper level in the
>> call stack as well. In this particular case, it needs to be used in
>> both `get_library` and `load` to address the original issue.
>
> Correct. It needs to be used wherever Django is catching an exception
> arising from user-supplied code and then raising a different
> exception.
>
>> 3. Design -- Seeing the ImportError's traceback is indeed helpful in
>> identifying the source of the problem. But, it's also kind of weird  
>> to
>> have the traceback of an ImportError (ending with the line "import
>> notthere" or whatever) associated with a different exception.
>
> I'll just clarify for anyone that hasn't applied the patch and tried
> it: with the patch in place, the traceback will still show down to the
> TemplateSyntaxError, but instead of stopping there will also continue
> down to where the ImportError originated. Yes, it's a little weird;
> Python 3.0 handles this whole situation much better with "raise ...
> from ..." which is intended for use in precisely these situations.

`wrap_and_raise()` will appear in the traceback, `raise  
wrap_exception(SomeException())` would be cleaner.

Better yet, make all exceptions that are used to reraise other  
exceptions a subclass of WrappingException (pick a better name) that  
either takes a `cause=exc` or `wrap=True` kwarg. This way, you don't  
have to add imports everywhere.

Your patch seems to swallow the new exception's traceback now instead  
of the causing traceback. That might be good in some situations, but  
generally both should be preserved.
__
Johannes

--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




Re: #7539 (ON DELETE support) aka ORM-09 - A call for feedback

2009-12-07 Thread Johannes Dollinger
Ping.

Since it's a non-trivial patch and there has been (almost) no  
feedback, is it save to assume that #7539 is not in scope for 1.2 ?

Am 07.11.2009 um 01:31 schrieb Johannes Dollinger:

>
> There's a new patch on the ticket[1], based on Michael Glassford's
> work, that solves a few remaining issues and should be fully backwards
> compatible. I haven't painted the bikeshed yet, so the API still is
> `on_delete=CASCADE | SET_NONE | SET_DEFAULT | PROTECT | DO_NOTHING |
> SET(value)`. Two minor additions:
>
> `DO_NOTHING` does nothing. Let the db handle it or resolve the
>dependency in pre_delete (see: #10829 [2])
>
> `SET(value)` sets the foreign key to an arbitrary value. `value`
> may
>be a callable, `SET(None)` is equivalent to `SET_NULL`.
>
> To make `on_delete` work on m2m intermediary models
> `DeleteQuery.delete_batch_related()` had to go. Intermediary models
> now use (almost) the same related-objects-collection code path as
> every other model (thanks to m2m-refactor). Because that would have
> lead to lots of SELECT queries for related objects, I refactored the
> collection algorithm to collect batches of objects instead of
> individual objects. That reduced the overhead to
> `#INTERMEDIARY_INSTANCES / GET_ITERATOR_CHUNK_SIZE` queries. This
> refactoring has a nice side-effect: Given the following code
>
> class A(models.Model): pass
> class B(models.Model): a = models.ForeignKey(A)
> class C(models.Model): b = models.ForeignKey(B)
>
> a = A.objects.create()
> for i in xrange(100): B.objects.create(a=a)
> a.delete()
>
> the `delete()` call results in 103 queries with trunk, and only 4
> queries with my patch applied.
>
> Finally, collecting related objects for auto_created intermediary
> models is short-circuited to avoid the extra SELECTs completely. The
> same could be done for any model that has no related objects, if we
> didn't need the instances to send signals (Someday/Maybe:
> Meta.send_signals = bool or tuple).
>
> Since the constants used for `on_delete` are just callables, it's
> possible to do any kind of calculation to decide what should happen to
> related instances, e.g.:
>
> def delete_or_setnull(collector, field, objects):
> setnull = []
> cascade = []
> for obj in objects:
> if can_delete(obj):
> delete.append(obj)
> else:
> setnull.append(obj)
> SET_NULL(collector, field, setnull)
> CASCADE(collector, field, cascade)
>
> fk = ForeignKey(To, on_delete=delete_or_setnull, null=True)
>
> This should probably not be documented at first, but it would be a
> nice feature once it's clear the on_delete handler signature will
> remain stable.
>
>
> FIXME:
> * I'd like to introduce `DatabaseFeatures.can_defer_constraint_checks`
> to disable nulling out foreign keys when it's not necessary. This
> would save a couple of UPDATE queries.
>
> * There are ugly contrib.contenttypes imports in
> `DeleteQuery.delete_batch_related()`. I left all contenttypes related
> code there (just renamed the method, it's still called). Someday/Maybe
> this could be removed and handled as a custom `on_delete` argument on
> GenericForeignKey.
>
> * There are no docs.
>
>
> Any feedback welcome.
>
> @glassfordm: If you're still working on this patch, I'd like to hear
> what you think (and get those tests you mentioned). I'm sorry I
> somewhat hijacked your work.
>
> __
> Johannes
>
> [1] http://code.djangoproject.com/ticket/7539
> [2] http://code.djangoproject.com/ticket/10829
>
>
>
>
> --~--~-~--~~~---~--~~
> You received this message because you are subscribed to the Google  
> Groups "Django developers" group.
> To post to this group, send email to django- 
> develop...@googlegroups.com
> To unsubscribe from this group, send email to 
> django-developers+unsubscr...@googlegroups.com
> For more options, visit this group at 
> http://groups.google.com/group/django-developers?hl=en
> -~--~~~~--~~--~--~---
>


--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




Re: Multiple database support: Request for feedback and testing

2009-12-05 Thread Johannes Dollinger

Am 05.12.2009 um 06:36 schrieb Russell Keith-Magee:
> [...]
>> What if the admin was instead fixed
>> by providing facilities for the more general case outlined above?
>>
>> What would this look like?  I'm picturing another setting (bleh) that
>> maps apps and/or models to specific databases.  Name choices aside,
>> this might look something like:
>>
>> DATABASE_USING = {
>> 'django.contrib.auth': 'default',
>> 'myapp': 'database1',
>> 'myapp2': {
>> 'Model1': 'database2',
>> 'Model2': 'database3',
>> }
>> }
>>
>> The admin could automatically take advantage of this setting to
>> determine what database to use for a given model.
>
> Alex, Joseph Kocherhans and I discussed this exact idea at Djangocon.
> You are correct that it has a lot of potential uses - not only the
> admin, but also loaddata/dumpdata, syncdb, and anywhere else that an
> iteration over models is required.
>
> However, it's a little bit more complicated than you make out.
>
> This sort of setting is very easy to give as a 10 line example, but in
> practice, this isn't what you will require - you will effectively need
> to duplicate the contents of INSTALLED_APPS. I have projects in the
> wild with dozens of entries in INSTALLED_APS - all running on a single
> database. Writing a DATABASE_USING setting like the one you describe
> would be extremely cumbersome and annoying.
>
> So, we could hijack INSTALLED_APPS to represent the idea of database
> deployment, using tuples/dictionaries rather than strings to define
> how apps are deployed. However, this comes at the cost of backwards
> compatibility for anyone iterating over INSTALLED_APPS.
> [...]

Let me propose a different colored pattern. It's backwards compatible,  
dry, feels like the urlconf approach and may evolve into a fix for  
#3591:

INSTALLED_APPS = (
app('django.contrib.auth', using=...),
app('myapp'),
app('myapp2', models=(
model('Model1', using=...),
model('Model2', using=...),
),
)

Where `app()` would look like this:

APP_USING = {}

def app(path, **kwargs):
if 'using' in kwargs:
APP_USING[path] = kwargs['using']
...
return path

The downside: It would require an import in settings.py. And the  
global dict is not exactly beautiful - but the price for bc.

This is not really an argument for a settings based solution (+0). But  
if there will be one, don't make it nested tuples/dicts - the  
existance of tuple literals in python does not make s-expressions a  
natural representation of configuration.
__
Johannes

--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




Re: What's the expected behavior of (cached) instances of deleted objects?

2009-11-26 Thread Johannes Dollinger

Am 26.11.2009 um 12:09 schrieb Russell Keith-Magee:

> On Thu, Nov 26, 2009 at 5:39 PM, Johannes Dollinger
> <johannes.dollin...@einfallsreich.net> wrote:
>>
>> Am 26.11.2009 um 03:47 schrieb Russell Keith-Magee:
>>
>>> On Thu, Nov 26, 2009 at 12:04 AM, Johannes Dollinger
>>> <johannes.dollin...@einfallsreich.net> wrote:
>>>> QuerySet.delete() currently sets the primary key and all nullable
>>>> foreign keys (to deleted objects) of instances passed to signal
>>>> handlers to None. No cache is updated.
>>>>
>>>> Model.delete() will do the same, but as these instances are  
>>>> collected
>>>> by traversing related object descriptors all reverse o2o field  
>>>> caches
>>>> will be updated.
>>>
>>> Are you sure there is a discrepancy here?
>>>
>>> ModelBase.delete() creates a CollectedObjects() structure, then
>>> invokes self._collect_sub_objects() to populate it. It then invokes
>>> delete_objects() to delete the objects.
>>>
>>> QuerySet.delete() creates a CollectedObjects() structure, then  
>>> invokes
>>> obj._collect_sub_objects() on each of the objects in the queryset to
>>> populate. It then invokes delete_objects() to delete the objects.
>>
>> With QuerySet.delete() all instances traversed by
>> _collect_sub_objects() are only accessible from signal handlers,  
>> there
>> will be no surprises.
>> But with Model.delete() you can possible access the same instances
>> through reverse o2o descriptors.
>> Here is an example: http://dpaste.com/hold/125343/
>
> Looks to me like you've found some edge case errors. If I'm reading
> the test case right, the o2o field isn't nullable, so you should be
> getting None for c.pk, and DNE on every next/prev request.

There's currently no code that would update forward foreign key caches.
So it's the edge case that does the updates (reverse o2o) and the  
general case just leaves your object graph alone (including stale  
caches).

 class A(models.Model): pass
 class B(models.Model): a = models.ForeignKey(A)

 a = A.objects.create()
 b = B.objects.create(a=a)
 b.a.delete()

Although both instances have been deleted, `b.a` will still be  
accessible.
__
Johannes

--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




What's the expected behavior of (cached) instances of deleted objects?

2009-11-25 Thread Johannes Dollinger
QuerySet.delete() currently sets the primary key and all nullable  
foreign keys (to deleted objects) of instances passed to signal  
handlers to None. No cache is updated.

Model.delete() will do the same, but as these instances are collected  
by traversing related object descriptors all reverse o2o field caches  
will be updated.

Is this an accidental side effect or desired behavior?

I'm asking because my patch for #7539 currently does not update o2o  
caches. But that could be fixed - if really necessary.

Relevant thread: 
http://groups.google.com/group/django-developers/browse_thread/thread/6e88b6315f489403/

__
Johannes

--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.




Re: Template Caching - Ticket #6262

2009-11-16 Thread Johannes Dollinger

Am 16.11.2009 um 15:31 schrieb Russell Keith-Magee:

> On Mon, Nov 16, 2009 at 10:04 PM, Alex Gaynor  
>  wrote:
>> On Mon, Nov 16, 2009 at 8:58 AM, Russell Keith-Magee
>>  wrote:
>>> On Thu, Nov 12, 2009 at 9:15 AM, Mike Malone   
>>> wrote:
 Sup,

 I've been working on template caching (ticket #6262) with a mister
 Alex Gaynor. The patch is getting pretty stable and I think it's  
 close
 to merge-able, so if anyone wants to take a look at what we've  
 got and
 provide feedback: go!
> ...
>>>  * On the subject of specifying loaders - in order to use the cached
>>> loader, we need to import the cached loader, and have support for
>>> callable loaders in find_template_loader. Requiring imports in the
>>> settings file seems like a bit of a wart to me.
>>>
>>> Here's an alternate proposal - rather than allowing callables, how
>>> about allowing entries in TEMPLATE_LADERS to be tuples, and
>>> interpreting the [1:] elements of the tuple as the arguments to the
>>> loader - for example:
>>>
>>> TEMPLATE_LOADERS = (
>>>('django.template.loaders.cached', (
>>>'django.template.loaders.filesystem.loader',
>>>'django.template.loaders.app_directories.loader',
>>>)
>>>)
>>> )
>>>
>> I think this could get fairly complicated quickly, for example is  
>> this valid?
>>
>> TEMPLATE_LOADERS = (
>>('django.templaet.loaders.cached', (
>>('django.template.loaders.filesystem', (
>>'/home/alex/django/templates/',
>>)
>>)
>> )
>
> Sure. Why not? The intention of what you have written seems pretty
> clear to me (typo and missing brackets notwithstanding :-).

Couldn't you just make this explicit (and more readable)?

{{{
# settings.py
TEMPLATE_LOADERS = ('project.template_loaders.loader',)
}}}

{{{
# template_loaders.py
from django.template.loaders.cached import Loader as CachingLoader
from django.template.loaders.filesystem import Loader as  
FileSystemLoader

loader = CachingLoader([
FileSystemLoader([
'/path/to/templates',
]),
])
}}}

If I read the patch correctly, that should already be possible and  
solves arbitrarily complex initialization for loaders.
If there has to be a convenience addition, how about a simple boolean  
`WRAP_TEMPLATE_LOADERS_WITH_CACHING_LOADER=True#FIXME: find a better  
name`?

__
Johannes





--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=.




Re: #7539 (ON DELETE support) aka ORM-09 - A call for feedback

2009-11-12 Thread Johannes Dollinger

Am 11.11.2009 um 17:50 schrieb Michael Glassford:

>
> Johannes Dollinger wrote:
>>
>> Am 10.11.2009 um 17:22 schrieb Michael Glassford:
>>
>>> I haven't had a chance to look at the patch yet, but what you  
>>> describe
>>> here sounds good. I don't have any problem with you "hijacking" the
>>> work.
>>>
>>> Did your patch deal at all with the unit tests in my patch that
>>> intentionally failed to expose things that weren't working right  
>>> yet?
>>
>> Only the first of your patches contains tests.
>
> Oops, right you are. The unit tests were unintentionally omitted. I've
> uploaded a new patch which is the pretty much the same as my previous
> patch but with my unit tests included (although comparing the two  
> now I
> see a few other differences, probably because I also updated to the
> latest code in SVN).
>
>> But the ON DELETE
>> related tests in this patch that do not test database-level support
>> should be covered by my tests in modeltests/on_delete/.
>
>> Which of your tests failed?
>
> The tests that fail are in tests/modeltests/on_delete_django/tests.py.
> They have names like test_issue_1a, test_issue_1b, test_issue_2a,
> test_issue_2b, etc. The comments on the tests indicate the expected
> result. Once the issues they test for are fixed, they should all pass.

Thanks a lot. I ran your tests with my patch. There were a few  
(harmless) failing tests:
- sanity checks for SET_NULL on fields that are not nullable or  
SET_DEFAULT on fields that have no default value. I moved the code to  
model validation (and added tests there), so I just removed these tests.

- you assumed SET_NULL as the default behavior for nullable FKs. That  
would be backwards incompatible. I changed the default to CASCADE, so  
I had to update related tests.

The remaining failing tests are test_issue_2a, -_2b, -_2d, -_2e. They  
test if related object caches are updated/cleared after `delete()`,  
e.g.:

 data = Data.objects.create(data=1)
 m = M.objects.create(fk=data) # fk is a nullable ForeignKey
 data.delete()
 self.assertEqual(None, m.fk) #Fails

I don't think this is required. #17[1] was just rejected in the 1.2  
voting process, and without identity mapping (or a global object  
collection or excessive use of signals) you would not be able to cover  
all use-cases. If you called Data.objects.all().delete() instead of  
data.delete() you'd still have outdated related object caches. Or if  
you had obtained m through M.objects.get(..). Or ..

I'm aware that django currently *does* null out PKs of deleted objects  
as well as nullable references to those objects (which will only be  
present in objects that have themselves been deleted). But it's  
limited to instances reachable from Model.delete() via reverse one-to- 
one descriptors and instances passed to signal handlers.
And finally: it's neither documented nor tested (you can remove the  
relavant four lines from delete_objects() without breaking existing  
tests).

My patch actually changes the behavior of Model.delete() in this  
regard: reverse one-to-one caches are not updated.

I'd rather see model instances as a "checkout" from the db that can  
get out of sync. Are there any deeper problems with this approach?

[1] http://code.djangoproject.com/ticket/17
__
Johannes

--

You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=.




Re: #7539 (ON DELETE support) aka ORM-09 - A call for feedback

2009-11-10 Thread Johannes Dollinger


Am 10.11.2009 um 17:22 schrieb Michael Glassford:

>
> I haven't had a chance to look at the patch yet, but what you describe
> here sounds good. I don't have any problem with you "hijacking" the  
> work.
>
> Did your patch deal at all with the unit tests in my patch that
> intentionally failed to expose things that weren't working right yet?

Only the first of your patches contains tests. But the ON DELETE  
related tests in this patch that do not test database-level support  
should be covered by my tests in modeltests/on_delete/.

Which of your tests failed?

__
Johannes


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Call for feedback: First GSoC patch ready to land

2009-10-30 Thread Johannes Dollinger

The m2m-refactor patch breaks ManyToManyFields on abstract models:  
"AssertionError: ForeignKey cannot define a relation with abstract  
class A"

{{{
class R(models.Model): pass

class A(models.Model):
 r_set = models.ManyToManyField(R, related_name="%(class)s_set")

 class Meta:
 abstract = True

class B(A): pass
class C(A): pass
}}}

Disabling `create_many_to_many_intermediary_model()` if  
`cls._meta.abstract` fixed it for me.

__
Johannes







--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Adding signing (and signed cookies) to Django core

2009-10-24 Thread Johannes Dollinger


Am 25.09.2009 um 22:04 schrieb Simon Willison:

>
> On Sep 25, 4:44 pm, Johannes Dollinger
> <johannes.dollin...@einfallsreich.net> wrote:
>> Regarding parity, let me advertise a Signer object again:
>>
>> signer = signed.Signer(
>> key=settings.SECRET_KEY,
>> expire_after=3600
>> )
>>
>> s = signer.sign(v)
>> v = signer.unsign(s)
>>
>> signer.set_cookie(response, name, value)
>> signer.get_cookie(request, name)
>> # or:
>> signer.get_cookies(request)[name]
>>
>> # transparently sign cookies set via response.set_cookie()
>> # and unsign cookies in request.COOKIES in place:
>> @signer.sign_cookies
>> def view0(request):
>> ...
>> @signer.sign_cookies('cookie', 'names')
>> def view1(request):
>> ...
>>
>> This would make more options and customization feasible (by
>> subclassing):
>
> OK, you got me. You obviously know my weakness for customisation by
> subclassing :) Having a Signer class is a smart idea, I'll kick it
> around a bit and see how it looks.


http://dpaste.com/hold/111386/

This is how a class-based API might look. It's based on django-signed.
All cookie related code is untested.

__
Johannes




--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: django.template refactoring (again) #7806

2009-09-29 Thread Johannes Dollinger


Am 29.09.2009 um 04:07 schrieb Russell Keith-Magee:

>
> On Tue, Sep 29, 2009 at 12:44 AM, Johannes Dollinger
> <johannes.dollin...@einfallsreich.net> wrote:
>>
>>>> Variable and FilterExpression will be deprecated as they require  
>>>> the
>>>> caller to know the length of the expression in advance.
>>>
>>> I'm not sure I follow this assertion. I have no problem believing
>>> FilterExpression can be made cleaner, but Variable takes an  
>>> expression
>>> like 'article.section', plus a context, and resolves it into a  
>>> string.
>>> In itself, it seems like a well built and stable piece of API.
>>
>> To get this  expression string in the first place you have to split
>> Token.contents somehow. smart_split() does that well for space
>> delimited expressions.
>> But if you try to parse something like {% url %} you have to split  
>> its
>> results again: "foo.bar,baz|filter:arg,42" won't do it. The current  
>> {%
>> url %} parser now resorts to `str.split(",")` and
>>{% url viewname list|join:"," %}
>> will break it. Of course that's a corner case and could be fixed with
>> something like `smart_split(delim=",")`. But it would be much more
>> elegant if you could just read an expression from a stream of tokens.
>> That's what I meant when I said "the {% url %} parser is broken by
>> design". I didn't mean to sound harsh - {% url %}'s parser is a
>> reasonable tradeoff between simplicity and correctness.
>
> Sure - that explains why FilterExpression is a problem, but I don't
> see why Variable is affected. Once you've parsed your expression into
> tokens, those tokens that are known to be variables need to convert
> 'foo.bar' into something derived from context.

The idea was that in most cases the template tag doesn't care if you  
pass in a Variable or a FilterExpression (#5756). So ideally there was  
only one method that returns something with a .resolve(Context)  
method. I know that Variable has to remain functional - and it is.  
It's a 20 lines wrapper around the TokenStream API: 
http://github.com/emulbreh/django/blob/master/django/template/compat.py#L8

>> It's both: making the API friendlier and fixing bugs (by using the
>> higher level API).
>> I thought that motivation and scope were mostly clear and approved.
>> Let me refer you to the two previous threads for now:
>>
>> http://groups.google.com/group/django-developers/browse_thread/thread/fb246422fa4e23d8
>> http://groups.google.com/group/django-developers/browse_thread/thread/4f5b01719e497325
>
> Herein lies the problem. The message you appear to have taken from
> those threads doesn't match with what I see. I see:
>
> * Agreement that work is needed on templates
> * A long debate about a blog post comparing Jinga and Django's  
> templates.
> * Some very specific comments from Malcolm addressing your approach
> to backwards compatibility [1]
> * Agreement that this is a very large body of work
> * Agreement that the API for custom tags is too verbose.
> * Comments that the process of fixing bugs should be independent of
> improving API [2]
>
> [1] http://groups.google.com/group/django-developers/msg/97d32ac923b1f700
> [2] http://groups.google.com/group/django-developers/msg/100e70150f37c172
>
> On these points, you won't get any argument from me.
>
> What I don't see is any specific endorsement of your approach, beyond
> a general "looks like you're going the right way" comment from Jacob
> [3]. However, that comment could be leveled at _any_ attempt to
> rewrite the parser - after all, it's the parser that is the problem.
> Your approach specifically _doesn't_ address Malcolm's concerns [2].

My approach was: create a new API for TagParsers then use this API for  
all tag libraries in django. All bug fixes just fell out of this. I  
don't see the point in fixing those bugs with the old API first and  
then throwing away the code. And I understand that backwards  
compatibility is important, there exist no backwards compatibility  
issues I know of (FilterExpression behaviour was the only major  
problem).

> [3] http://groups.google.com/group/django-developers/msg/6e03c7c1a5b3d946
>
> Your code also seems to be a moving target. My original impression was
> that you were calling for a major review of a potentially trunk-ready
> patch. Deeper reading revealed that there were potentially lots of
> backwards incompatibilities. Reading subsequent messages reveals that
> maybe those backwards incompatibilities don't exist any more. At this
> point, I'm straight up confused as to the state of this patch - is 

Re: django.template refactoring (again) #7806

2009-09-28 Thread Johannes Dollinger

>> Variable and FilterExpression will be deprecated as they require the
>> caller to know the length of the expression in advance.
>
> I'm not sure I follow this assertion. I have no problem believing
> FilterExpression can be made cleaner, but Variable takes an expression
> like 'article.section', plus a context, and resolves it into a string.
> In itself, it seems like a well built and stable piece of API.

To get this  expression string in the first place you have to split  
Token.contents somehow. smart_split() does that well for space  
delimited expressions.
But if you try to parse something like {% url %} you have to split its  
results again: "foo.bar,baz|filter:arg,42" won't do it. The current {%  
url %} parser now resorts to `str.split(",")` and
{% url viewname list|join:"," %}
will break it. Of course that's a corner case and could be fixed with  
something like `smart_split(delim=",")`. But it would be much more  
elegant if you could just read an expression from a stream of tokens.  
That's what I meant when I said "the {% url %} parser is broken by  
design". I didn't mean to sound harsh - {% url %}'s parser is a  
reasonable tradeoff between simplicity and correctness.

> As best as I can make out, you are correct. FilterExpression appears
> to be an internal, so isn't covered by our backwards compatibility
> guarantees. However, Variable is documented API.

Variable and FilterExpression would still be around - compatible  
versions live in `django.template.compat` - and import paths remain  
functional.

>> 1.) Currently `Variable("unresolvable")` and
>> `FilterExpression("unresolvable", p)` resolve differently: Variable
>> raises VariableDoesNotExist and FilterExpression returns
>> TEMPLATE_STRING_IF_INVALID. Except when `ignore_failures=True` is
>> given. But `FilterExpression("foo|cut:unresolvable", p)` will again
>> raise VariableDoesNotExist - regardless of `ignore_failures=True`.
>>
>> My Expression implementation unifies these behaviours: If any part of
>> an expression is unresolvable a LookupError will be raised.
>> `ignore_failures` will be deprecated but there's a resolve_safe()
>> method on the Expression base class that reads:
>>
>> def resolve_safe(self, context, default=None):
>> try:
>> return self.resolve(context)
>> except LookupError:
>> return default
>>
>> This would be a small backwards incompatible change. I have one
>> failing test (exception02) because the ExtendsNode implementation
>> depends on the current FilterExpression behaviour.
>
> It's either backwards compatible or it isn't. Backwards compatibility
> is a requirement, not an optional extra.

If FilterExpression is not part of the public API this is a non-issue.  
The problem was that unresolvable variables were treated differently  
from unresolvable filter arguments.

>> 3.) Numeric literals ending with "." are currently treated as
>> Variables. The following test (ifequal-numeric07) succeds because
>> `ignore_failures=True` in IfEqualNode suppresses the inevitable
>> VariableDoesNotExist exception and then compares None to 2: ('{%
>> ifequal x 2. %}yes{% endifequal %}', {'x': 2}, ''). My current
>> implementation raises a TemplateSyntaxError as soon as it  
>> encounters a
>> numeric literal with a trailing dot. This could easily be made
>> backwards compatible if it's considered worth it.
>
> Sorry, but this one can't change. "2." is a tested format for handling
> floats; the fact that this is inconvenient to your parser is
> irrelevant.

The problem is probably in bug in Variable:

 >>> from django.template import Variable, Context
 >>> Variable("2.").resolve(Context())
Traceback (most recent call last):
...
raise VariableDoesNotExist("Failed lookup for key [%s] in %r", (bit,  
current)) # missing attribute
 >>> Variable("2.").resolve(Context({"2": {"": "foo"}}))
'foo'

I originally wanted to parse "2." as a numeric literal. But there's  
currently code that explicitly attempts to reject such literals: 
http://code.djangoproject.com/browser/django/trunk/django/template/__init__.py#L663

And here's how I fixed it in my branch (as mentioned in my second post  
in this thread): 
http://github.com/emulbreh/django/commit/73bc93b4dbf251968e062825439033de33597b08

>> * {% url ... %}, {% ssi ... %} currently accept unquoted literal
>> strings. This will continue to work but the preferred syntax will use
>> properly quoted strings. This may one day allow for viewnames and ssi
>> paths that come from expressions.
>
> At this point, this sort of fix can't happen until v2.0. It's would be
> a major change at this point. It's nice to know that the template
> parsing tools will handle it when the time comes, but I would expect
> that to be true of _any_ rework of the parser.

The proposed change is fully backwards-compatible. Both tags only gain  
a new syntax variant. By documenting the unquoted variant as  
deprecated means that newly written templates will 

Re: django.template refactoring (again) #7806

2009-09-25 Thread Johannes Dollinger

Ping. Anyone?


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Adding signing (and signed cookies) to Django core

2009-09-25 Thread Johannes Dollinger


> Would expire_after on the unsign just automatically imply
> timestamp=True? There's been a lot of concern raised about parity in
> the API, and it reads a little weird with the two different arguments.
> I'm not sure it's a problem, but it's just a little funny.

Regarding parity, let me advertise a Signer object again:

signer = signed.Signer(
key=settings.SECRET_KEY,
expire_after=3600
)

s = signer.sign(v)
v = signer.unsign(s)

signer.set_cookie(response, name, value)
signer.get_cookie(request, name)
# or:
signer.get_cookies(request)[name]

# transparently sign cookies set via response.set_cookie()
# and unsign cookies in request.COOKIES in place:   
@signer.sign_cookies
def view0(request):
... 
@signer.sign_cookies('cookie', 'names')
def view1(request):
...

This would make more options and customization feasible (by  
subclassing):
  - put signatures in separate cookies (or a single separate cookie)
  - automatically include request.user.id (to prevent users from  
sharing cookies)
  - swap out the hash, serialization, or compression algorithm without  
changing the token format
  - customize when and how expiration is determined

__
Johannes


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Adding signing (and signed cookies) to Django core

2009-09-24 Thread Johannes Dollinger

How about Signer class?

signer = Signer(key=...)
assert signer.unsign(signer.sign(value)) == value

This way you wouldn't have to pass around key, extra_key, and  
potential further arguments but a single Signer instance. Plus, you  
could easyly overwrite hashing, concatenation, and serialization as  
well as add functionality transparently, eg.:

sig = TimestampedSignatureFactory(ttl=3600) # sig.unsign() will raise  
SigntureExpired after 3600 seconds

> 1) request.unsign_cookie('foo') -- This breaks the parallelism with
> existing cookies.  Sometimes we'll be doing request.COOKIES['foo'] and
> sometimes we'll be doing request.unsign_cookie('foo').
>
> 2) A decorator for views -- @unsign_cookies("foo", "bar") -- This
> doesn't allow any sort of fall-back (you can't customize what to do if
> a given cookie is improperly signed)
>
> 3) COOKIES as an intelligent object -- We can overload .get so we're
> doing something like request.COOKIES.get('foo', signed=True) -- I
> think this has the best chance at an interface that keeps a consistent
> feel. It's completely backward compatible, though it breaks the
> traditional expectation of what you can do via the `get` method on a
> dictionary.

4) A signed.Cookies object:

signed_cookies = signed.Cookies(request, key=...)
signed_cookies.set_cookie(key, value, **kwargs)
value = signed_cookies[key]

or

signed_cookies = signer.get_cookies(request)
...

__
Johannes





--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Sequence Literals in Templates

2009-09-07 Thread Johannes Dollinger

I'd like to propose python-like tuple syntax for templates and  
grouping brackets for filter expressions.

{% for x in ("default", var, f|exp) %} ... {% endfor%}
{{ x|yesno:("yes", "no", "maybe") }}
{% something (1,2,3,4)|slice:(start, stop) %}
{% if x|contains:(foo|last) %} ... {% endif %}

This could be used to fix #1199 (Supporting more than one argument in  
a custom filter) but argc checks would have to be part of the filter  
implementation (possibly some kind of @unpack_filter_args decorator).

I have a working patch on top of my template refactoring branch[1] at 
http://dpaste.com/hold/90575/
It contains a backwards compatible, exemplary change for |yesno.

[1] http://github.com/emulbreh/django/tree/master



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: django.template refactoring (again) #7806

2009-09-07 Thread Johannes Dollinger

django.template.defaulttags, django.template.loader_tags,  
django.templatetags.i18n, django.templatetags.cache,  
django.contrib.comments are now converted.
All tests pass. Mostly unmodified:

-# Basic filter usage with space between a variable and  
filter pipe
-'filter-syntax03': ("{{ var |upper }}", {"var": "Django  
is the greatest!"}, "DJANGO IS THE GREATEST!"),
+# Raise TemplateSyntaxError for space between a variable  
and filter pipe
+'filter-syntax03': ("{{ var |upper }}", {},  
template.TemplateSyntaxError),

-# Basic filter usage with space after a filter pipe
-'filter-syntax04': ("{{ var| upper }}", {"var": "Django  
is the greatest!"}, "DJANGO IS THE GREATEST!"),
+# Raise TemplateSyntaxError for space after a filter pipe
+'filter-syntax04': ("{{ var| upper }}", {},  
template.TemplateSyntaxError),

Fixed tickets (incomplete list):
#4746: Allow whitespace before and after filter separator
#5862: Filters are not applied in an include tag [closed: wontfix]
#6271: filter arguments with spaces break several template tags
#6510: get_nodes_by_type refactor for easier extensibility (fixes an  
underlying bug for IfEqualNode)
#6535: parser.compile_filter() does not support negative numbers
#7377: "extends" and "block" tags are not available when constructing  
template from scratch
#9757: Make IfNode a baseclass for other if nodes to subclass

> * {% if not %} would fail with my current implementation. This is
> fixable. But I'm not sure it's worth it: not/and/or should be keywords
> in the context of boolean expressions.


I hacked around this issue (the test now passes), but it's not exactly  
beautiful.
The same is true for the ifequal-numeric07 bug: numeric literals with  
a trailing dot will now be resolved as None.

If there is interest, I could clean up those commits once more. This  
was my first exposure to git - and there is some sloppiness related  
ugliness:
http://github.com/emulbreh/django/commits/master

Johannes


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



django.template refactoring (again) #7806

2009-08-28 Thread Johannes Dollinger

The proposal and motivation are essentially the same as in the last  
thread [1] and the ticket description [2]. I put it on the 1.2 feature  
list.
I tried to split my patch into smaller, more readable commits here:  
http://github.com/emulbreh/django/tree/master
I haven't converted defaulttags, loadertags and django.templatetags.*  
yet to show that it's (mostly) backwards compatible (see below). It  
would be useful to decide on some questions in the "API Design .."  
section below first.

I'll summarize the core ideas again as well as problems and pending  
design decisions.

Concepts: Expressions and TokenStream
=
Variable and FilterExpression will be deprecated as they require the  
caller to know the length of the expression in advance. This makes  
whitespace handling cumbersome and some syntactical constructs really  
hard to parse (e.g., the {% url ... %} parser is broken by design).  
Instead there would be a tokenizer (currently named TokenStream) that  
lets you read expressions from a stream of tokens - this would replace  
Token.split_contents().

 >>> bits = TokenStream(p, 'url views.client_action  
id=client.id,action="update",num=3').tokens; bits
[('name', 'url'), ('name', 'views.client_action'), ('name', 'id'),  
('char', '='), ('name', 'client.id'), ('char', ','), ('name',  
'action'), ('char', '='), ('string_literal', '"update"'), ('char',  
','), ('name', 'num'), ('char', '='), ('numeric_literal', '3')]

Now, instead of explicitly constructing a FilterExpression object you  
would simply call `bits.parse_expression()` and would get back an  
object with a `.resolve(context)` method (a subclass of  
django.template.expressions.Expression) that represents a Literal,  
Variable (called Lookup), or FilterExpression. For compatibility  
issues see "Variable vs FilterExpression" below.

A TokenStream can be read via the following api (the current stream  
position is called `offset`).
Low level API
---
``def pop(self)``
Returns a pair (tokentype, lexeme) and offset+=1; tokentype is one of
"string_literal", "numeric_literal", "char", "name".
``def pop_lexem(self, match)``
Returns True and offset+=1 if the next token's lexeme equals `match`.
Returns False otherwise.
``def pop_name(self)``
Returns the next token's lexeme and offset+=1 if its tokentype is
"name". Returns None otherwise.
``def pushback(self)``
offset-=1
High level API

These methods raise TokenSyntaxError and leave offset untouched if
the expected result cannot be parsed.
Each accepts a boolean required=False kwarg which turns
TokenSyntaxErrors into TemplateSyntaxErrors if True.
``def parse_string(self, bare=False)``
Returns the value of the following string literal. If bare=True,
unquoted strings will be accepted.
``def parse_int(self)``
Returns the value of the following numeric literal, if it is an int.
``def parse_value(self)``
Returns an Expression that evaluates the following literal, variable
or translated value.
``def parse_expression(self)``
Returns an Expression that evaluates the following value or
filterexpression.
``def parse_expression_list(self, minimum=0, maximum=None, count=None)``
Returns a list `e` of expressions; minimum <= len(e) <= maximum.
count=n is a shortcut for minimum=maximum=n.


Variable vs FilterExpression
==
I could only find documentation for Variable. If the internally used  
Parser.compile_filter() is indeed undocumented there is no official  
way to use FilterExpression in custom tags. If that means that  
FilterExpression.resolve() behaviour doesn't have to be backwards  
compatible, that would help a lot ..

1.) Currently `Variable("unresolvable")` and  
`FilterExpression("unresolvable", p)` resolve differently: Variable  
raises VariableDoesNotExist and FilterExpression returns  
TEMPLATE_STRING_IF_INVALID. Except when `ignore_failures=True` is  
given. But `FilterExpression("foo|cut:unresolvable", p)` will again  
raise VariableDoesNotExist - regardless of `ignore_failures=True`.

My Expression implementation unifies these behaviours: If any part of  
an expression is unresolvable a LookupError will be raised.  
`ignore_failures` will be deprecated but there's a resolve_safe()  
method on the Expression base class that reads:

 def resolve_safe(self, context, default=None):
 try:
 return self.resolve(context)
 except LookupError:
 return default

This would be a small backwards incompatible change. I have one  
failing test (exception02) because the ExtendsNode implementation  
depends on the current FilterExpression behaviour.

2.) Currently `Variable("a._hidden")` works - but  
`FilterExpression("a._hidden", p)` raises a TemplateSyntaxError. This  
would be unified: lookup parts may not start with an underscore. If  
it's not acceptable to break bc here leading underscores might simply  
be allowed.

3.) Numeric literals ending with "." are currently 

Re: Default manager

2008-12-15 Thread Johannes Dollinger

I like the `objects` convention as well as explict default manager  
declarations.
I proposed both in 
http://groups.google.at/group/django-developers/browse_thread/thread/3e1a668ac84328b6/ce36cbe46276d807
 
.

Advertisement:

My radical "solution" is to never modify the initial QuerySet. And I  
use exactly one manager per model.
The example for my "one manager should be enough for everyone"- 
approach from the thread above now lives here: http://dpaste.com/hold/99386/ 
  .

A DRYer version:
{{{
class MusicianQuerySet(PersonQuerySet):
 @proxy_manager_method
 def guitarists(self):
 return self.filter(instrument='guitar')

class Musician(Person):
 instrument = models.CharField(max_length=50)
 objects = QuerySetProxyManager(MusicianQuerySet)()
}}}

So, why do you need more than one manager per model?
Or: why is Post.live.all() better than Post.objects.live()?




--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Template-04's scope

2008-12-04 Thread Johannes Dollinger


>> However, that's no reason not to have a better API on top that
>> third-party tags can use -- as long as internal tags don't break.
>
> That should be entirely possible, using either an additional decorator
> for parser functions [EMAIL PROTECTED] or [EMAIL PROTECTED]($new_api=True)`, 
> or
> a new method [EMAIL PROTECTED] (for some reasonable value of
> $new_api).

Even better, we could use class based parsers: http://dpaste.com/96291/
That would make reusing parsers much simpler.



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Template-04's scope

2008-11-25 Thread Johannes Dollinger


Am 25.11.2008 um 19:45 schrieb Steve Holden:

>
> Jacob Kaplan-Moss wrote:
>> On Tue, Nov 25, 2008 at 10:11 AM, Johannes Dollinger
>> [...]
>>> * I always felt the API for custom tags is too verbose.
>>>
>>
>> There's almost universal agreement on this point :)
>>
>> It's unfortunately pretty tricky to do in a backwards-compatible way:
>> we've sorta pasted ourselves into a corner here by letting tags do
>> anything they want in terms of parsing. This lead to an inconsistency
>> in syntax that we (unfortunately) have to continue to support.
>>
>> However, that's no reason not to have a better API on top that
>> third-party tags can use -- as long as internal tags don't break.
>>
> Heretical suggestion: {: :}, {! !}, {$ $} or {[ ]} with a different
> processor (falling back to the original if necessary), and therefore  
> no
> need for a fully backwards-compatible approach?

That hopefully won't be necessary. However, the possibility to  
configure those token boundaries at runtime might be a nice feature. I  
missed it for LaTeX templates.


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Template-04's scope

2008-11-25 Thread Johannes Dollinger


Am 25.11.2008 um 17:28 schrieb Jacob Kaplan-Moss:

> I'm actually pretty happy with the general idea you've taken here --
> django.template needs some cleanup, and a holistic approach that
> addresses the multiple issues at once is going to produce better code
> than multiple patches all stacked up.
>
> It seems most of the core team agrees -- it got 7 +0 votes. I think
> the only reason it failed to get any +1s is that it's just a big
> amount of work to review all at once. There's also the obvious
> question of weather we can pull off this refactoring without breaking
> backwards-compatibility.

I'm glad you like it, more on backwards-compatibility below.

> I think the best thing to do with this is to take it to a branch so we
> can easily evaluate it against the trunk templates. If you're down for
> working on this off on a branch, I'm willing to help out and also
> review your code.
>
>> * The code shuffling is purely cosmetic, it just feels wrong to stuff
>> everthing in __init__.py. It's already too big for my taste. And I
>> honestly don't know how to cleanly split shuffling and functionality
>> in separate patches.
>
> Yeah, this is the big reason a branch would be better -- we can
> separate the big deck-chair-shuffling from the refactoring and be able
> to review better.
>
>> * I always felt the API for custom tags is too verbose.
>
> There's almost universal agreement on this point :)
>
> It's unfortunately pretty tricky to do in a backwards-compatible way:
> we've sorta pasted ourselves into a corner here by letting tags do
> anything they want in terms of parsing. This lead to an inconsistency
> in syntax that we (unfortunately) have to continue to support.
>
> However, that's no reason not to have a better API on top that
> third-party tags can use -- as long as internal tags don't break.

That should be entirely possible, using either an additional decorator  
for parser functions [EMAIL PROTECTED] or [EMAIL PROTECTED]($new_api=True)`, or 
 
a new method [EMAIL PROTECTED] (for some reasonable value of  
$new_api).
It might even be possible to fold `TokenStream` into `Token`, but that  
would introduce state in `Token` - haven't thought this through.

The refactored version of defaulttags currently passes most tests with  
the following exceptions
* {% if not not %} raises TemplateSyntaxError (fixable, but I'm not  
shure it's worth it)
* integer literals with a trailing colon raise TemplateSyntaxError
* whitespace is insignificant, therefore {{ foo | filter }} passes.

The only real bc problem were unquoted unicode viewnames in {% url %}.  
That's fixed in my latest patch at the expense of regexp beauty.
The current version accepts a quoted string as well, so one could  
deprecate unquoted viewnames and fix that in 2.x.
That's been rejected in 
http://groups.google.com/group/django-developers/browse_thread/thread/fd30cb20f80c6c79
 
.

> So: what do you say we make a template-cleanup branch and start
> working on this over there?

If it has to be a branch, so be it. I entered myself as implementor  
into the feature table.




--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Template-04's scope

2008-11-25 Thread Johannes Dollinger

I'm aware that #7806 propably does too much at once. I'd like to hear  
what's in scope for 1.1 and what you believe doesn't belong in django.

Why the patch still does that/too much:

* The code shuffling is purely cosmetic, it just feels wrong to stuff  
everthing in __init__.py. It's already too big for my taste. And I  
honestly don't know how to cleanly split shuffling and functionality  
in separate patches.

* I always felt the API for custom tags is too verbose. There's almost  
no abstraction which makes parsing something like {% url %}  
unnecessarily hard - even django's builtin parser doesn't get it right  
(split(' ') and split(',')). Yes, that could be seen as a feature  
request. But builtin tags would also benefit from abstraction. Just  
comparing code length (compactness is desirable, although by no means  
the only quality measure): #7806's defaulttags contains 20% less LOC  
(not counting blank lines, comments, and docstrings).

* The bugs, obviously. As Malcolm pointed out, most are symptoms of  
the same underlying problems.

Anyway, here's a list of the tickets I tried to fix in #7806 (although  
some of them were merely side effects):

Bugs


#5756: Any template tag that uses Variable instead of  
parser.compile_filter does not handle filters.
http://code.djangoproject.com/ticket/5756

#5862: Filters are not applied in an include tag
Wontfixed, but it's really an instance of #5756.
http://code.djangoproject.com/ticket/5862

#5971: django.template.TokenParser inconsistent with parsing filters
http://code.djangoproject.com/ticket/5971

#6271: filter arguments with spaces break several template tags
http://code.djangoproject.com/ticket/6271

#6296: ifequal, filter and hardcoded-string doesn't work
http://code.djangoproject.com/ticket/5756

#6510: get_nodes_by_type refactor for easier extensibility (fixes an  
underlying bug for IfEqualNode)
http://code.djangoproject.com/ticket/6510

#6535: parser.compile_filter() does not support negative numbers
http://code.djangoproject.com/ticket/6535

#7295: quotes, escaping and translation of string literals handled  
inconsistently in templates
http://code.djangoproject.com/ticket/7295

#7377: "extends" and "block" tags are not available when constructing  
template from scratch
http://code.djangoproject.com/ticket/7377

#9315: Keyword arguments with spaces and the url tag
http://code.djangoproject.com/ticket/9315


Features
===

#4746: Allow whitespace before and after filter separator
The patch goes further: whitespace is insignificant anywhere between  
tokens. That means
`{% url view foo , kw = bar | filter : arg %}`
would be valid.
http://code.djangoproject.com/ticket/4746


Threads
===
There are a couple more I'm aware of and probably and probably dozens  
I haven't found yet - as a starting point:
http://groups.google.com/group/django-developers/browse_thread/thread/4f5b01719e497325
http://groups.google.com/group/django-developers/browse_thread/thread/ba0644b835e3ec40
http://groups.google.com/group/django-developers/browse_thread/thread/c90b6e29d20724ca

Related to the code shuffling in #7806 (although my intention was  
merely readability):
http://groups.google.com/group/django-developers/browse_thread/thread/35aae0e8da804bd9






--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Proposal: Let Context handle template loading (#7815)

2008-09-24 Thread Johannes Dollinger

I should have made this more accessible ..

http://code.djangoproject.com/ticket/2949
"I want to be able to set the path to a directory that contains the  
templates I want to render at runtime. [...] The basic thing I'm  
looking for is to be able to load a template from a specific,  
arbitrary path and then tags like extends and include should load  
templates from the same directory."

http://code.djangoproject.com/ticket/3544
"Fix {% include %} to allow recursive includes" - ConstantIncludeNode  
is the problem.

http://code.djangoproject.com/ticket/4278
"get_template should accept a dirs argument" - `dirs` kwargs could  
still be useful, but `render_to_*(... , dirs=)` could be handled with  
a custom `loader` for `context_instance`.

http://code.djangoproject.com/ticket/6834
"Add support for templates to be loaded from dynamically-selected  
directories" - a more focused version of #2949.

http://code.djangoproject.com/ticket/7931
"Debug mode causes template includes to be parsed regardless of if  
tags" - yet again, ConstantIncludeNode is the problem.

--
Out of scope for now but mentionable: This could be used for simple  
marcos if Context allowed external population of its cache:

{% sub "braces.sub" %}
 {% templatetag openbrace %} {{ content }} {% templatetag  
closebrace %}
{% endsub %}

{% include "braces.sub" %}
or with #7817:
{% include "braces.sub" with foo as content %}


Am 24.09.2008 um 16:34 schrieb Johannes Dollinger:

>
> #7815[1]:
>
> * Adds a loader kwarg to Context which should be something with a
> get_template(name) function. Default would be django.template.loader.
> * Provides get_template() and select_template() methods on Context.
> These are used in {% include %}, {% extends %} and inclusion_tag.
> * Caches templates in Context.
> * Removes ConstantIncludeNode since caching provides the same
> benefits and it causes problems with recursive includes.
>
> An example:
> {{{
> # emulate select_template for include/extends:
> class PrefixLoader:
>  def __init__(self, prefix_list):
>  self.prefix_list = prefix_list
>  def get_template(name):
>  from django.template.loader import select_template
>  return select_template(['%s/%s' % (prefix, name) for prefix
> in self.prefix_list])
>
> tpl.render(Context({}, loader=PrefixLoader(['a', 'b', 'c'])))
> }}}
>
> This would fix #2949, #3544, #4278, #6834, and #7931. But it's a
> backwards incompatible change: If you rely on compile time side
> effects (e.g. {% load %}) in included templates, that will break.
>
> Opinions?
>
> [1] http://code.djangoproject.com/ticket/7815
>
>
> >



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Optimizations on templates

2008-09-24 Thread Johannes Dollinger


Am 25.09.2008 um 00:39 schrieb Manuel Saelices:

>
> It's a different aproach. I consider my cache attempt a optimization
> like URL dispatcher cache, without considering django cache system,
> like url resolvers cache, _join_cache in orm or other many cases.
> Also, I'm not sure that #6262 consider the inheritance problem (look
> at copy.deepcopy(self.get_parent()) line in my patch).

Your approach requires django.template.loader to be reloaded whenever  
a template changes, which probably means a server restart.
This is different from other module level caches in django because  
those are only invalidated if other python modules are modified  
(urls.py or models.py).

I'm not familiar with either patch and I not shure I understand the  
problem you tried to fix with that deepcopy().
Anyway, from a quick look: you damaged {% extends var_name %}, and  
this new `dirs` argument to `get_template()` should be part of the  
cache key, and what does `from_child` do?


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Proposal: Let Context handle template loading (#7815)

2008-09-24 Thread Johannes Dollinger

#7815[1]:

* Adds a loader kwarg to Context which should be something with a  
get_template(name) function. Default would be django.template.loader.
* Provides get_template() and select_template() methods on Context.  
These are used in {% include %}, {% extends %} and inclusion_tag.
* Caches templates in Context.
* Removes ConstantIncludeNode since caching provides the same  
benefits and it causes problems with recursive includes.

An example:
{{{
# emulate select_template for include/extends:
class PrefixLoader:
 def __init__(self, prefix_list):
 self.prefix_list = prefix_list
 def get_template(name):
 from django.template.loader import select_template
 return select_template(['%s/%s' % (prefix, name) for prefix  
in self.prefix_list])

tpl.render(Context({}, loader=PrefixLoader(['a', 'b', 'c'])))
}}}

This would fix #2949, #3544, #4278, #6834, and #7931. But it's a  
backwards incompatible change: If you rely on compile time side  
effects (e.g. {% load %}) in included templates, that will break.

Opinions?

[1] http://code.djangoproject.com/ticket/7815


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Concrete django.template Suggestions

2008-09-21 Thread Johannes Dollinger


Am 19.09.2008 um 18:08 schrieb Armin Ronacher:
[snip]
>
> What's harder to fix is how the i18n integration translates filter
> arguments or gettext constants (those _("foo") thingies).  Currently
> that happens very often at node/object creation rather than node/
> object evaluation.  A good example for that problem is
> FilterExpression.__init__.  The translation would have to be moved to
> the resolve method in that case.  When would a language change between
> template compilation and rendering?  If the language changes each
> request which is a very common case on multilingual websites.

My patch treats _(x) as syntactic sugar for x|gettext - where  
`gettext = lambda x: x and ugettext(x) or u''`.

> Changing the parser to a more advanced system or making the template
> engine more consistent is not so important for the time being, but I
> want to raise a few situations I encountered where the behaviour is
> confusing:
>
>   - cycle arguments are delimited by spaces and each item can be an
> expression (however if there is a comma involved somewhere, it
> seems like the tag is interpreted as comma separated list of
> strings
> which makes the 'cycle "foo", "bar", "baz"' yield unexpected
> results.
>   - On the other hand arguments for the url tag are comma delimited,
> but whitespace after comma is not allowed.
>
>   - The group-by part of the regroup tag is an expression, but
> everything
> else but a variable name returns unexpected results.  Furthermore
> does this tag only work on dicts.  By the group-by part I'm
> refering
> to the expression after the "by" keyword:
>
>   {% regroupy foo by bar as blah %}
>
> bar is here treated as constant string, even though it's
> represented
> as variable in the regroup node and in the syntax.
>
>   - Likewise ssi excepts one argument that looks like a variable, but
> is
> treated as constant string.
>
> Tags to be excluded from the variable-as-string rule should be block
> and load because they represent neither variables nor strings.  The
> argument for block works like a label and the argument for load is a
> library.

I agree, a convention for tuples and literal strings would be really  
helpful. I had to work around the {% url %} viewname issue (that's  
why TokenStream.parse_string() takes bare=True). It's solved  
backwards compatible yet accepts properly quoted strings. Whitespace  
may occur between tokens but is discarded. {% regroup %} uses  
TokenStream.parse_expression(), so that should be fixed too. {% cycle  
%} is problematic because it accepts the legacy {% cycle a,b as c %}  
format but that should be curable.


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: RFC: django.template refactoring (#7806)

2008-09-17 Thread Johannes Dollinger

> Would @register.tag(token_stream=True) work instead, or am I missing
> something?

Yes, that would work. 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



RFC: django.template refactoring (#7806)

2008-09-16 Thread Johannes Dollinger

Why should django.template be refactored? Filter and Variable parsing  
is inconsistent. Many ad hoc parsers in defaulttags are fragile.  
Whitespace handling is ungraceful.

The patch provided in #7806[1] splits __init__.py into separate  
modules[2] and introduces a TokenStream class that allows parsing of  
literals, vars (called lookups) and filter expressions.

Here's how it would work:

@register.tag
@uses_token_stream
def mytag(parser, bits):
 expr = bits.parse_expression(required=True)
 return MyNode(expr)

`uses_token_stream` replaces the Token argument to the parser  
function with a TokenStream object.
If the token is not fully parsed, a TemplateSyntaxError is raised.
For better examples, have a look at the patched version of  
`django.template.defaulttags`.


TokenStream API (first stab)

``def __init__(self, parser, source)``
parser: a django.template.compiler.Parser instance
source: a string or a django.template.compiler.Token instance

TokenStream tokenizes its source into "string_literal",  
"numeric_literal", "char", and "name" tokens, stored in self.tokens  
as (type, lexeme) pairs. Whitespace will be discarded.
You can "read" tokens via the following methods, the current position  
in self.tokens is maintained in self.offset.

A "char" token is a single character in ':|=,;<>!?%&@"\'/()[]{}`*+-'.  
A name is any sequence of characters that does contain neither "char"  
characters nor whitespace and is not a string or numeric literal.

 >>> TokenStream(parser, r'"quoted\" string"|filter:3.14 as  
name').tokens
[('string_literal', '"quoted\\" string"'), ('char', '|'), ('name',  
'filter'), ('char', ':'), ('numeric_literal', '3.14'), ('name',  
'as'), ('name', 'name')]
 >>> TokenStream(parser, r' "quoted\" string" | filter : 3.14  as  
name').tokens
[('string_literal', '"quoted\\" string"'), ('char', '|'), ('name',  
'filter'), ('char', ':'), ('numeric_literal', '3.14'), ('name',  
'as'), ('name', 'name')]

Low level API
-
``def pop(self)``
Returns a pair (tokentype, lexeme) and offset+=1; tokentype is one of  
"string_literal", "numeric_literal", "char", "name".

``def pop_lexem(self, match)``
Returns True and offset+=1 if the next token's lexeme equals `match`.  
Returns False otherwise.

``def pop_name(self)``
Returns the next token's lexeme and offset+=1 if its tokentype is  
"name". Returns None otherwise.

``def pushback(self)``
offset-=1


High level API
--
These methods raise TokenSyntaxError and leave offset untouched if  
the expected result cannot be parsed.
Each accepts a boolean required=False kwarg which turns  
TokenSyntaxErrors into TemplateSyntaxErrors if True.

``def parse_string(self, bare=False)``
Returns the value of the following string literal. If bare=True,  
unquoted strings will be accepted.

``def parse_int(self)``
Returns the value of the following numeric literal, if it is an int.

``def parse_value(self)``
Returns an Expression that evaluates the following literal, variable  
or translated value.

``def parse_expression(self)``
Returns an Expression that evaluates the following value or  
filterexpression.

``def parse_expression_list(self, minimum=0, maximum=None, count=None)``
Returns a list `e` of expressions; minimum <= len(e) <= maximum.  
count=n is a shortcut for minimum=maximum=n.


I'm unhappy with the naming of TokenStream and uses_token_stream  
(using_token_stream?). And I just noticed the english spelling of  
lexeme has a trailing "e".


[1] http://code.djangoproject.com/ticket/7806
[2] See [1] for details. Yes, this is orthogonal to the concern of  
the ticket. I gave up splitting the patch when I stumbled upon the  
first circular dependency issue - a half-assed effort - and decided  
it's not worth it.


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Is URL template tag's syntax going to change?

2008-07-20 Thread Johannes Dollinger


Am 20.07.2008 um 21:48 schrieb Malcolm Tredinnick:
>> Especially unquoted non-ascii strings (as in test url05) are ugly
>
> Subjective; not a technical reason. Non-ASCII strings look just the  
> same
> as ASCII strings. In fact, many non-Latin scripts are much prettier  
> than
> the Latin alphabet (just as subjective as your opinion, of  
> course :-)).

Of course that's subjective, everything is.

>> and
>> would require hacks or re.LOCALE in my approach in #7806.
>
> That would suggest a problem in the approach in that patch. :-)
>
> There's more than one template tag in Django that treats unquoted
> strings of characters as non-literals, although the url tag may be the
> only one at the moment where non-ASCII strings are fairly natural. We
> shouldn't rule them out, though. That needs to be handled by any
> template parsing code. Non-ASCII strings are just as easy to handle as
> ASCII strings in code. If you're at the point of thinking you need to
> use re.LOCALE, it suggests you're trying to be too strict in the error
> checking and loosening that up a bit would be faster, lead to  
> easier to
> read code and avoid difficulties like this.

Afaics the only other tag that would accept an arbitrary unquoted  
literal is {% ssi %}.
Are there more?
Variables are currently matched with [\w.] (at least in filter  
expressions) and that's what I use to match keywords and vars.

bit_re = re.compile(r"""
 (?P"(?:[^"\\]*(?:\\.[^"\\]*)*)"|'(?:[^'\\]*(?:\\. 
[^'\\]*)*)')
|(?P\b[+-]?\.?\d[\d\.e]*\b)
|(?P[\w.]+) # keyword or variable
|(?P\S) # punctuation
""", re.VERBOSE)
tokens = [(bit.lastgroup, bit.group(0)) for bit in bit_re.finditer 
(contents)]

Seems like I have to rethink this then.



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: Is URL template tag's syntax going to change?

2008-07-20 Thread Johannes Dollinger

I'd prefer treating unquoted parameters as variables. If that's not  
possible make url accept a quoted view name and deprecate unquoted  
view names.

Especially unquoted non-ascii strings (as in test url05) are ugly and  
would require hacks or re.LOCALE in my approach in #7806.

Am 20.07.2008 um 20:56 schrieb Malcolm Tredinnick:

>
>
> On Sun, 2008-07-20 at 17:16 +0400, Ivan Sagalaev wrote:
>> Julien Phalip wrote:
>>> Hi,
>>>
>>> For months, there has been a mention in the url tag's documentation:
>>>
>>> "Note that the syntax for this tag may change in the future, as we
>>> make it more robust." [1]
>>>
>>> Is that mention still relevant for 1.0 and beyond?
>>
>> If I recall correctly, there was a discussion that the first  
>> parameter
>> should be changed to work like most other parameters to tags:  
>> treat it
>> literally if it's quoted, otherwise look it up in context. The  
>> objection
>> was that potential benefit wasn't worth the hassle of a backwards
>> incompatible change. But I can't remember if there was a final  
>> decision.
>
> The couple of times I brought it up, it died from a lack of core
> developers really caring that much. I thought it would have been a  
> good
> idea to change it originally. I think it would be a bad idea to change
> it now, since it's been in use for so long. If somebody wants to use
> variables in a url-like tag, it's trivial to write your own variant.
> We're not hurting anybody's software too much by not making the  
> change,
> although it would have been nice to do so for consistency.
>
> I suspect that unless Adrian or Jacob pipe up in the very near future
> with agreement to change it, we can remove that note because the  
> current
> API will be set in stone.
>
> Malcolm
>
>>
>>>
>>
>
>
> >



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: about templatetag namespaces

2008-07-11 Thread Johannes Dollinger

+1 for the main proposal and subproposal (modulo concrete syntax).  
Decoupling template library imports from app_label would also be good  
for #3591. Get rid of as many app_label dependencies as possible.  
Beautiful languages such as php and c have flat namespaces - don't go  
there.
Concerning your subsubproposal: strictly only {% load %} would be  
essential. So the separation would be mostly arbitrary. Just allow  
libraries to shadow existing tags/filters.

(For those dismissing django.template features because of non- 
programmer/designer/beginner use-cases: there already are quite a few  
possibilities to shoot yourself in the foot, and I'd bet the template  
language is turing complete.)

Am 10.07.2008 um 17:08 schrieb Amit Upadhyay:

> Hi,
>
> I have identified a few issues over the time that makes working  
> with custom django template tags/filters a little less than ideal  
> for me.
>
> My chief problem is: if I see an unknown/unfamiliar tag/filter and  
> want to locate the exact code responsible, I have to do the  
> following: find all loads defined in the template, for each of  
> those "loaded template libraries", I have to find the py file by  
> searching it in all INSTALLED_APPS/templatetags folder and then  
> scanning them for a register_tag/register_filter call with the  
> unknown name. This to me is a little bit too much magical[PS1].  
> Further, what if there are more than one apps with some_template.py  
> in their templatetags folder? And why is it called templatetags  
> when it can have both filters and tags? (*tongue in cheek remark :-).
>
> Bottom line is clean namespacing is difficult to achieve as of now,  
> but would be possible with the following "syntax":
>
> {% load mytemplatelib.few_tags as cool_tags %}
>
> and used subsequently as {% cool_tags.show_something %}. Or the  
> alternate syntax:
>
> {% load mytemplatelib.few_tags.* %} and I can do {% show_something  
> %} [this sytax would come with its warning similar to the ones  
> issued against using the "from django import *" like imports in  
> python].
>
> Note: mytemplatelib.few_tags would be search in sys.path and not in  
> installed_apps' templatetags folders.
>
> As a template writer I will have to provide a module  
> mytemplatelib.few_tags with a show_something named function which  
> acts as a tag. There would be some decorators/protocol to tell if  
> they are valid tag/filter.
>
> Assuming the idea is accepted, It can be implemented in a mostly  
> backward compatible fashion, post 1.0, either by having a {% import  
> %} named new loader for 1.x, or even {% load %} name can be  
> preserved as one can first look into the old way of loading and if  
> it fails can look for new way.
>
> This has been discussed before[1], but I have some objections with  
> the latest implementation[2]:
> * appname/templatetags still persist, and is the place where  
> libraries will be looked into, there is no reason for django to  
> enforce and create its own import paths, PYTHONPATH, sys.path etc  
> are good enough for that. Let application developers and  
> templatetags library writer pick what makes sense. Looking into  
> INSTALLED_APPS constraint does not buy much and can be cleaned up  
> entirely.
> * We still have to do the search if we get a new/unfamiliar  
> template tag/filter. This is just not pythonic to me.
>
> Both these are backward incompatible changes, but like I said  
> current way can be kept, with some DepricationWarning, and 1.x may  
> include the new way which looks for module name in sys.path instead  
> of app_name/templatetags/.
>
> Subproposal:
>
> Add settings: DEFAULT_TEMPLATE_EXTENSIONS [avoiding the words tags/ 
> filter as it can be either], being a list of strings that will be  
> automatically be {% load whatever %} in all templeates.
>
> Subsubproposal:
>
> Split defaultta

> gs into defaulttags_essentials which will include tags like "load",  
> "extends", "block" etc, which are really required for templates to  
> work vs others like "for"/"if" and the rest into some other module  
> that gets added by default in  
> "settings.DEFAULT_TEMPLATE_EXTENSION". So that if I can use the  
> fancier versions of my template tags/filters by turning off the  
> django ones and using just mine, without having to use any  
> namespace prefix, and without having to worry abt conflicts with  
> django's default template tags/filters.
>
> PS1: Though there is nothing stopping me from writing a script that  
> takes the tag/fitler name and returns me the qualified name of the  
> function handling the tag/filter.
> [1]: http://groups.google.com/group/django-developers/browse_frm/ 
> thread/2b5a4a31f0349d27/b92c96a8dccc214b#b92c96a8dccc214b
> [2]: http://code.djangoproject.com/attachment/ticket/2539/ 
> patch_2539_code_tests.diff
>
> -- 
> Amit Upadhyay
> Vakow! www.vakow.com
> +91-9820-295-512
> >



--~--~-~--~~~---~--~~
You received this message because 

django.template feature proposals

2008-06-18 Thread Johannes Dollinger

I'd like to propose the following changes to django.template. Some of  
them are partially or fully implemented in my patch[1] for #3544.
Which of these have a chance to land in 1.0 (assumed I provide a  
patch, docs and tests)?

1.) Allow recursive includes, #3544 [2]

2.) Add a template_cache to `Context`, accessible via  
Context.get_template and Context.select_template. Use this in  
IncludeNode, ExtendsNode, and inclusion_tag.

3.) Add a `loader` kwarg to Context to force use of a specific  
templateloader (something with a get_template() method). This would  
fix #2949 [3].
class PrefixLoader:
 def __init__(self, prefix_list):
 self.prefix_list = prefix_list
 def get_template(name):
 from django.template.loader import select_template
 return select_template(['%s/%s' % (prefix, name) for prefix  
in self.prefix_list])

tpl.render(Context({}, loader=PrefixLoader(['a', 'b', 'c'])))
This emulates select_template() for include and extends nodes.

4.) Extend with-tag syntax to allow

{% with foo as x and bar as y and ... %}
and/or {% with foo as x, bar as y, ... %}

5.) Extend include-tag syntax to allow

{% include "template" with foo as x and bar as y %}
and/or {% include "template" with foo as x, bar as y %}

This would be equivalent to {% with ... %}{% include ... %}{% endwith  
%}.

6.) Add an optional `separator_chars` argument to  
utils.text.smart_split and template.Token.split_contents that enables  
extra delimiters

 >>> map(str, smart_split('a, b,c ,d, e ,f', separator_chars=','))
['a', ',', 'b', ',', 'c', ',', 'd', ',', 'e', ',', 'f']
 >>> map(str, smart_split('a=1, b=2 , c = 3', separator_chars=',='))
['a', '=', '1', ',', 'b', '=', '2', ',', 'c', '=', '3']

and use this in url, cycle, with, etc.

7.) Add a {% sub "name" %}...{% endsub %} tag that populates  
Context.template_cache["name"] with its contained nodelist. {%  
include "name" %} would then be used to render it.

[1] http://code.djangoproject.com/attachment/ticket/ 
3544/3544.contexttemplatecache.2.diff
[2] http://code.djangoproject.com/ticket/3544
[3] http://code.djangoproject.com/ticket/2949


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: The Model.default_manager concept

2008-06-16 Thread Johannes Dollinger

The QuerySet method examples [1] mostly use the corresponding Manager  
proxy method.
Probably QuerySet.create() exists to use querysets where managers are  
expected.

An ugly corner case:

cat.article_set.filter(...).create(title=title)

is equivalent to

Article.objects.create(title=title)


[1] http://www.djangoproject.com/documentation/db-api/#queryset- 
methods-that-do-not-return-querysets


Am 16.06.2008 um 21:06 schrieb Ken Arnold:

>
> True. But surprisingly enough, the `create` method is a QuerySet
> instance method. And it doesn't use any of the filtering, so
>
> Article.objects.filter(category=cat).create(title=title,
> content=content)
>
> doesn't do what you'd expect. (Though `cat.article_set.create` should
> work.) Has that actually confused anyone?
>
> -Ken
>
>
> On Jun 16, 2:57 pm, Johannes Dollinger
> <[EMAIL PROTECTED]> wrote:
>>> So then what is the difference between a Manager and a QuerySet?
>>
>>> Nearly everything would work identically if Manager were simply:
>>
>>> class Manager(QuerySet):
>>> pass
>>
>>> (except actually keeping the magic that connects it to the model
>>> class.)
>>
>> Utility methods in managers wouldn't make much sense if Manager was a
>> QuerySet:
>>
>> User.objects.filter(username='foo').create_user('bar',
>> '[EMAIL PROTECTED]')
>>
>> Although those utilities could as well be class methods.
> >



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: The Model.default_manager concept

2008-06-16 Thread Johannes Dollinger


Am 16.06.2008 um 20:56 schrieb James Bennett:
> It'd also hurt the reusability of managers (which is a big advantage I
> and others have taken advantage of), because you wouldn't be able to
> keep methods specific to a single model separate from methods which
> aren't, at least not without introducing a whole chain of manager
> classes inheriting from each other to bring in the right sets of
> methods.

That's exactly what I do now - I have model specific managers and use  
them as bases for submodel managers.
There are no reusability drawbacks and you can split functionality as  
needed.
You could add a manager_mixin attribute to Meta to make the parallel  
inheritance structure (models and managers) more convenient.




--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: The Model.default_manager concept

2008-06-16 Thread Johannes Dollinger

> So then what is the difference between a Manager and a QuerySet?
>
> Nearly everything would work identically if Manager were simply:
>
> class Manager(QuerySet):
> pass
>
> (except actually keeping the magic that connects it to the model
> class.)

Utility methods in managers wouldn't make much sense if Manager was a  
QuerySet:

User.objects.filter(username='foo').create_user('bar',  
'[EMAIL PROTECTED]')

Although those utilities could as well be class methods.


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: The Model.default_manager concept

2008-06-16 Thread Johannes Dollinger


Am 16.06.2008 um 18:49 schrieb James Bennett:

>
> On Mon, Jun 16, 2008 at 9:44 AM, Johannes Dollinger
> <[EMAIL PROTECTED]> wrote:
>> If you're just want different querysets you can use something like
>> this: http://dpaste.com/53948/.
>
> Or I can use managers and also add other supporting methods (which I
> also often do).

You could as stick them in a single manager as well (and wouldn't  
have to remember which method is available via which manager).
My point is that one manager per model would be enough to do anything  
you can do with multiple managers (and if you want modified querysets  
you get a little extra flexibility if you just subclass QuerySet).  
Therefore (imho) the cleanest solution for the problem with  
_default_manager and model inheritance appears to get rid of multiple  
managers.
I don't really expect this to be accepted as it would break lots of  
existing code. But there should be a way to bypass the  
Manager.creation_counter magic.


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: The Model.default_manager concept

2008-06-16 Thread Johannes Dollinger

If you're just want different querysets you can use something like  
this: http://dpaste.com/53948/.

Things.live.all() vs Things.objects.live().

Am 16.06.2008 um 16:37 schrieb James Bennett:

>
> On Mon, Jun 16, 2008 at 9:22 AM, Johannes Dollinger
> <[EMAIL PROTECTED]> wrote:
>> Is there a rationale for multiple managers per model?
>
> Yes, and I at least use them all the time. For example, I'll often
> have one manager that does no special filtering, and another that only
> returns things with a "live" or "published" status. Once you start
> working with them, you find lots of uses for multiple-manager tricks
> like this.
>
>
> -- 
> "Bureaucrat Conrad, you are technically correct -- the best kind of  
> correct."
>
> >



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Re: The Model.default_manager concept

2008-06-16 Thread Johannes Dollinger

Second try. I promise there won't be a third.

Is there a rationale for multiple managers per model? Or is it a  
that's-how-it's-always-been thing?
If I missed something obvious, I would have received a "wrong list or  
rtfm" reply by now, supposedly.

{{{
class AManager(Manager):
 pass

class BManager(Manager):
 pass

class A(Model):
 a_objects = AManager()
 class Meta:
 abstract = True

class B(Model):
 b_objects = BManager()
 class Meta:
 abstract = True

class C(A,B):
 objects = Manager()

class D(B,A):
 objects = Manager()
}}}

Now C._default_manager == C.a_objects and D._default_manager ==  
D.a_objects.
If A and B come from different modules _default_manager will be  
picked depending on import order.

If this is by design, I'd be happy with a "wontfix" reply. I'm not  
using multiple managers anyway.

[1] http://code.djangoproject.com/ticket/7154



Am 31.05.2008 um 19:16 schrieb Johannes Dollinger:

>
> I'd like to propose a change of the Model.defaul_manager concept. My
> first concern was the inability to control the default_manager when
> subclassing a Model that already provides a default_manager (the base
> class' manager will be added before the subclass' manager).
>
> This could be solved by either an explicit default_manager
> declaration in Meta,
>
> my_manager = MyManager()
> class Meta:   
>   default_manager = 'my_manager'
>   
> or the convention that default_managers are called `objects` (then
> you wouldn't even need the default_manager concept).
> I strongly favour the latter, because it's dry and simple. Yet it's a
> backwards incompatible change ..
>   
> My attempts to come up with a good example to illustrate my proposal
> were in vain - and I begin to think, that it's always cleaner to keep
> a single Manager (and subclass as necessary) and never ever alter
> Manager.get_query_set() behaviour (or all() for that matter). Does
> anybody have a good use-case that depends on multiple managers?
>
> Here is how I would write the custom manager doc example (from [1]):
> http://dpaste.com/53948/ (Those proxy methods could use a little meta-
> programming.).
>
> Has this approach any flaws? If not, shouldn't the docs advertise
> this more flexible approach instead of multiple Managers?
>
> ___
> Johannes
>   
> [1] http://www.djangoproject.com/documentation/model-api/#custom-
> managers
>
>
> >



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



#3591: add support for custom app_label (and verbose_name?)

2008-06-12 Thread Johannes Dollinger

As proposed by mrts, I'd like to take the discussion from #3591 [1]  
here.

A note on InstalledAppsRevision [2]: I like most of it (especially  
`db_prefix`) but cannot see the value of having multiple instances of  
the same app. Is there an example that illustrates why this is needed  
(everything I can come up with is basically a design flaw)? And you  
would have to tell each 3rd party app with dependencies which  
instances to use.

Meta data in App: There will be lots of useful meta data (weight,  
description, style, short_description, icons, ...) cluttering App and  
preventing multiple AdminSite instances from using different meta  
data. I could accept verbose_name (if nfa doesn't remove it from  
fields), although it just doesn't feel right.
Something like `AppAdmin`[3] would play nice with multiple AdminSite  
instances and enable apps to provide default meta data (subclass it  
as needed).

I have another proposal which I originally wanted to defer until I  
could offer some code (and some form of App has landed in trunk):  
include app specific settings
   App('mypkg.myapp', 'myapp', settings = 'somepkg.settings.myapp')
accessible through `from mypkg.myapp.conf import settings`. I bring  
this up now, because it could be (ab)used to provide app meta data  
without cluttering App. Just reserve settings like _NAME,  
_DESCRIPTION, _ADMIN_WEIGHT, ..

Optionally: move settings.py to settings/__init__.py and look for  
settings/.py automagically.

[1] http://code.djangoproject.com/ticket/3591
[2] http://code.djangoproject.com/wiki/InstalledAppsRevision
[3] http://code.djangoproject.com/ticket/3591#comment:38


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Model.delete() vs QuerySet.delete() / Model.save() vs QuerySet.update()

2008-06-01 Thread Johannes Dollinger

Currently, QuerySet.delete() ignores custom Model.delete() methods  
(#6915 [1]). I'd like to discuss possible solutions - foo.delete()  
should be equivalent to Foo.objects.filter(pk=foo.pk).delete().

1. Disallow custom Model.delete() methods, offer signals only.
That's easy, but no real option IMO, as it prevents (pseudo-)atomic  
sideeffects (eg. delete a file).

2. Make QuerySet.delete() call Model.delete().
This would slow down QuerySet.delete() quite a bit for large  
QuerySets. But it would enable QuerySet.delete() on sliced QuerySets.
What should QuerySet.delete() do if any triggered Model.delete() call  
raises?

There is a symmetrical issue with QuerySet.update() and Model.save().  
This seems to be harder since it would require explicit UPDATE  
support [2] instead of "magic ORM save()" [3].

___
Johannes

[1] http://code.djangoproject.com/ticket/6915
[2] http://groups.google.com/group/django-developers/browse_thread/ 
thread/179ed55b3bf44af0
[3] http://groups.google.com/group/django-developers/browse_thread/ 
thread/bd67a65cd221c909


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---



Where should I start? (#2539, #3544, #6915, #7154, #7215, #7312)

2008-05-29 Thread Johannes Dollinger

Hello,

I'm quite new to django (first real project) and stumbled across several
bugs/features that I'd like to see in trunk. And I'm willing and  
hopefully able
to help. Here they are: #2539, #3544, #6915, #7154, #7215, #7312, and  
(cosmetic)
#5537.

The following should probably be split into different threads, but I  
did't feel
like spamming the list ..

1. Abstract base classes should be able to provide Managers (#7154).
   Then the first manager definition for each subclass will be the  
abstract base
   class' manager and therefore become the subclass' default manager  
(if it's not
   overwritten). Now you can either specify a different default  
manager or keep
   the base class' manager - you can't do both.
   I've not been bitten by this so far (not shure I ever will,  
because I stick to
   a single Manager and subclass as necessary), anyway:

   I'd suggest changing default manager resolution from "whichever  
comes first"
   to "whichever is called `objects`" - imho that would be much  
cleaner, even
   without the abstract base class usecase.

   I'm aware this is probably a big backwards-incompatible change,  
but it could
   be worth it, for the sake of clarity.


2. I would have expected custom Model.delete() methods to called  
whenever the
   ORM wants to delete objects in that model. However, QuerySet.delete 
() uses a
   single SQL statement and you only get a signal. This is #6915.

   So either prohibit custom delete() methods (a dont-call-delete- 
unless-you-
   really-mean-it policy) or make sure delete() is called every time.

   In the latter case QuerySet.delete() could easily be patched to  
work on sliced
   QuerySets too - and should probably rollback if any custom delete 
() call
   fails.


3. Namespaces. I really like namespaces. That's why I like #2539. And  
why I dis-
like the app_label concept. It's a crippled one-word namespace  
and makes me
think about save app_labels far to long (might there be a usefull  
3rd-party
app that happens to use this?). Proposal:

INSTALLED_APPS = (
AppConfig('django.contrib.auth', db_table_prefix = 'xauth'),
)

AppConfig could optionally take a `settings` keyword argument so  
custom
settings would not need a 'MYAPP_' prefix. This would obviously  
require an
api to retrieve those settings.


4. add_to_query() needs documentation. And tests. And a fix: see #7312.
I would start this, but after reading  
django.db.models.sql.query.Query, I'm
scared.

Where should I start?

__
Johannes


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---