On Tue, 2009-01-06 at 15:38 -0800, Killarny wrote:
> The above ticket was opened a while back concerning the inability to
> define views that use both positional arguments and keyword arguments
> when expecting to use reverse() to match urls tied to those views.
>
> I don't understand the rational here for not fixing this issue. The
> decision not to allow mixing args and kwargs seems like a lazy way to
> avoid coming up with a real solution. Forcing the use of one or the
> other but not both is simply not pythonic, and without some sort of
> discussion, seems silly. When a python framework does not allow valid
> python syntax to function properly, that sounds like a fundamental
> flaw in design to me.
Could you please be slightly less dismissive of the implementation
efforts? "Lazy", "silly", "not pythonic", "fundamental flaw" tend to
disregard the enormous efforts a number of people have put into making
that functionality work. We don't actually just make stuff up after a
few beers and commit the first thing that runs, you know. I don't think
it's unreasonable to ask you to assume the best of intentions on the
part of people implementing the code.
For the record, initially we tried very hard to allow mixed arguments.
SmileyChris and I had a lot of back and forth about it, since it would
have been potentially useful functionality. But it also wasn't critical
functionality. The reversing functionality is still fully usable without
that particular extra and, in the end, it turned out to be pretty
inefficient and made the code a lot more complex.
There's already a lot of processing and memory usage that goes into
making reversing work and increasing it further to allow for all the
possible mixed combinations of keyword and positional arguments (the
combinations grow at quite an alarming rate with a few optional
parameters involved) wasn't encouraging. Things were measurably slower
and reversing is already slow enough for that to be a concern. Plus,
everybody was having to pay the penalty, not just those using that
particular piece of functionality. Being able to split into only two
direct cases -- dictionary substitution or positional argument
substitution -- simplified a lot of things and removed a bunch of bugs
that were lurking in the edge cases.
I did have a version that almost worked with mixed argument types, but
even trying to document the behaviour so that people could predict which
of two patterns with differently named parameters that contained
optional pieces would be selected was brain-meltingly hard (some cases
led to basically arbitrary choices having to be made, which wasn't
predictable unless you knew how things were stored in an internal
dictionary and wouldn't have been portable). It wasn't doing any favours
to either the people who would have to use the function or those of us
who would have to field the bug reports and maintain it.
> There are many instances where, in a complicated implementation of
> views, one might want to have a combination of required args and
> optional kwargs, and the inability to mix them introduces all sorts of
> complexities to the logic of the views that shouldn't have to be dealt
> with.
I'll disagree with this. Whilst it's easy when one is faced with a
particular problem to imagine that it must be a common case, in reality
there aren't really that many instances where mixing is required (in
fact, I can't think of *any* where it could be compulsory -- it's purely
an alternative representation, so rewrites are always possible). There
are cases where it can be used, as you witness, but it's not *required*.
Porting isn't really that hard, either. It might well take a little
time, which is unfortunate, but it's not rocket surgery and it's a
one-off exercise. If you find you're needing to mix things, the most
straightforward change is to give all your parameters names and use
keyword arguments always.
> Due to the abruptness with which I was referred to this list, I feel
> like I must be missing some obvious piece of logic in this decision,
All that you're missing when you reopened the ticket is that we ask
people not to reopen tickets that have been closed as wontfix. In our
contributing document it asks you to come to the django-developers list
if you have further discussion. Otherwise things descend rapidly into a
seuqence of wontfix/reopen/wontfix/reopen just because people don't
agree on the decision. To be able to draw a close to that sort of
sequence, we ask that when a core developer closes a ticket as wontfix,
it doesn't get reopened. It's common on public bug tracking systems to
ask people not to just reopen tickets for precisely those reasons. Okay,
you didn't know that initially, which is why that sort of thing gets
cleared up pretty quickly. No problems there.
> as many of the other tickets have well defined responses and rational
> for the decisions related to them, while this ticket has almost none.
Maybe go back and reread the comments on the ticket, then, since you are
overlooking something. The first comment on the ticket (by me) explains
where the problems are. In abbreviated form, certainly, but there's
little point putting a huge essay into a ticket comment. If somebody
wants more details they can ask nicely and I'll explain, as has happened
here. The only reason I didn't wontfix it immediately when I wrote that
comment was because there was a documentation update I wanted to make.
I'm sorry that the resolution means you might have to rewrite some code,
because we do generally (and in this case in particular) put a lot of
effort, as developers, into causing the minimum amount of disruption and
inconvenience possible and you've just gotten unlucky in this
particular. However, the changes to reverse were necessary to fix some
bugs and avoid the introduction of others and were definitely not made
in haste. It also doesn't actually make any particular piece of
functionality impossible; you have to "spell" things a little
differently, but that's true with a lot of the porting process to Django
1.0. As noted, the previous behaviour only really worked by accident and
the current implementation provides a clear exception message when the
problem appears.
To end with some potential good news: you've missed a third solution to
your particular problem. If Django's builtin reverse() function and url
tag don't do what you want, write and use your own versions. They will
work for your particular situations and you will know their limitations
and what you can and can't do with them. Writing a url2 tag and doing a
global url -> url2 search-and-replace over your templates isn't going to
be a huge time sink. Django's url tag (more particularly, the reverse()
function) has to satisfy some particular requirements and every set of
requirements carries with it a constraints. If those constraints are too
restrictive for you, you can replace them with your own version with
it's own constraints quite easily. It just isn't appropriate for the
general case that Django's core is targeting.
In short, the decision was deliberate and neither lazy nor
short-sighted. It's not imposing a restriction on what you can do with
reverse() and if you prefer a different style of code, Django doesn't
require you to use the builtin reverse() or template tag. And... hey...
if you end up writing your own version that is simpler and faster and
works for all cases, then by all means post the patch in a new ticket
and we can review it and maybe include. We're never against improving
existing code, but as I hoped I've explained above, there is a pragmatic
side to what gets implemented, both for usage and maintenance (and they
aren't independent).
Best wishes,
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
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
