Code

Opened 6 years ago

Closed 4 years ago

#8764 closed (duplicate)

Documentation should explain you can't mix args and **kwargs in reverse()

Reported by: tolano Owned by: nobody
Component: Documentation Version: master
Severity: Keywords: reverse function exception
Cc: aribao@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by mtredinnick)

Changeset [8760] introduced this exception: "Don't mix *args and
kwargs in call to reverse()!"

django/trunk/django/core/urlresolvers.py
def reverse(self, lookup_view, *args, **kwargs): 
    if args and kwargs: 
        raise ValueError("Don't mix *args and **kwargs in call to 
reverse()!")


[8760] was suppose to be backwards compatible but is not. My question is:
Why is not possible to mis args and kwargs? I don't understand the
reason, and all of my reverse funcions will fail because of this.

I'll write an example of the problem. All my urls are prepared
following the best SEO rules, so in many places I have urls like:
(r'^(.*)-f(?P<id>\d+)\.html$','view_photo')

this would make the url:
my-photo-f2.html

I get the url using:

reverse('app.views.view_photo', args=[slugify(photo.title)], kwargs={'id':id})

(Note: I don't want to use a slug field.)

Now this won't work. Well, IMHO this is a bug. I don't see any reason why the reverse function shouldn't work the same way as it did before. Aplying a patch and make the funcion so limited is not appropiated.

Attachments (2)

0001-Doc-change-re-8764.patch (1.1 KB) - added by peterbraden 5 years ago.
Documentation patch clarifying situation
0002-Indentation-fix-in-docs.patch (746 bytes) - added by peterbraden 5 years ago.
Sorry - previous patch had incorrect indentation - this should fix

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by mtredinnick

  • Description modified (diff)
  • milestone 1.0 deleted
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

(Fixed description formatting)

It's pretty much accidental that that ever worked. There are cases where mixing positional and keyword arguments will given incorrect results in the previous code and even more so now (with optional groups). So, ok, there's an inadvertent backwards incompatibility for something that only worked by accident. It's simple to work around: pass in either just positional arguments or just keyword arguments. Both are easy enough to do.

Somebody will update the BackwardsIncompatibleChanges page at some point and we can add a docs note. For now, the error message is clear enough about what needs to be changed in anybody's code that was relying on this.

comment:2 Changed 6 years ago by SmileyChris

  • Resolution set to invalid
  • Status changed from new to closed

comment:3 Changed 6 years ago by SmileyChris

Actually, probably should have been a wontfix. Closed, nonetheless

comment:4 Changed 5 years ago by Killarny

  • Resolution invalid deleted
  • Status changed from closed to reopened

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.

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. Could anyone either fix this, or provide some sort of insight as to what the real rational here is?

comment:5 Changed 5 years ago by ubernostrum

  • Resolution set to wontfix
  • Status changed from reopened to closed

If you'd like to start a discussion on this, take it to the dev list. In the meantime, ticket stays closed.

comment:6 Changed 5 years ago by mtredinnick

For anybody else thinking of reopening this, please understand what comment:1 is saying. It's not merely difficult. It's impossible to do correctly for all cases and differentiating the cases where it's possible and the ones where it isn't is a computationally inefficient process.

comment:7 Changed 5 years ago by peterbraden

  • Component changed from Core framework to Documentation
  • Resolution wontfix deleted
  • Status changed from closed to reopened

I'm reopening, not because it needs to be fixed, but because the documentation actually is wrong when describing it:

Documentation reads:

{% url path.to.some_view arg1,arg2,name1=value1 %}

This is unclear, and the caveat described in the ticket should be explained in the docs.

Changed 5 years ago by peterbraden

Documentation patch clarifying situation

Changed 5 years ago by peterbraden

Sorry - previous patch had incorrect indentation - this should fix

comment:8 Changed 5 years ago by jacob

  • Summary changed from Mixing args and **kwargs in reverse() function to Documentation should explain you can't mix args and **kwargs in reverse()
  • Triage Stage changed from Unreviewed to Accepted

comment:9 Changed 5 years ago by vegas

I got directed here from the django-dev mailing list. I don't see anyone making an actual c'ase that this is impossible, and I'm fairly certain that it is quite possible. There once was a language called Prolog that approached all computation as a solution to this problem, and there is still a slobbering beast called XSLT that does it for every function call.

That being said, I'm not super familiar with the django source, so I buy that it might be an *engineering impossibility* at this point in time. I was debugging around in 1.1's guts yesterday, and noticed that it seems when an urlpattern with both positional groups and named groups is matched, the positional groups are dropped in the trash can. I don't think this is the Re modules fault, but if it were that would be a good case for letting this sleeping bug lie. I'll investigate the matter and post back here with my findings and hopefully a patch. Would love to hear any pointers anyone has.

comment:10 Changed 5 years ago by vegas

Apologies on the crazy bolding there. My g1 inserted a bunch of single quotes and has some serious issues with editing textboxes. Also, karen on the mailing list already pointed me at the google group thread on this topic

comment:11 Changed 4 years ago by timo

  • Resolution set to duplicate
  • Status changed from reopened to closed

duplicate of #12946

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.