[Django] #22218: Deprecate 'prefix' arg to django.conf.urls.patterns
Django
------------------------------------------------+------------------------
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (URLs) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
In the olden days of Django, it was encouraged to reference views as
strings in url patterns:
{{{
urlpatterns = patterns('',
url('^$', 'myapp.views.myview'),
)
}}}
and Django would magically import "myapp.views.myview" internally and turn
the string into a real function reference.
In order to reduce repetition when referencing many views from the same
module, the `patterns` function takes a required initial "prefix" arg,
which is prepended to all views-as-strings in that set of url patterns:
{{{
urlpatterns = patterns('myapp.views',
url('^$', 'myview'),
url('^other/$', 'otherview'),
)
}}}
In the modern era, we have updated the tutorial to instead recommend
actually importing your views module and referencing your view functions
(or classes) directly. This has a number of advantages, all deriving from
the fact that we are now using Normal Python in place of Django String
Magic: the errors when you mistype a view name are less obscure, IDEs can
help with autocompleting view names, etc.
With the advent of the class-based generic views, it is even more likely
that users are referencing view callables directly in their urlconf, due
to the need to call `.as_view()`.
So these days, the above use of the prefix arg is much more likely to be
written (and is better written) as:
{{{
from myapp import views
urlpatterns = patterns('',
url('^$', views.myview),
url('^other/$', views.otherview),
)
}}}
This leaves the 'prefix' arg in modern Django code as a useless empty
string that has to be passed to every invocation of `patterns`. This is an
ugly API wart, and a burden when teaching new users (answering the
newbie's question "why do I need this empty string as the first argument
to patterns?" requires either a time-wasting detour into the history of
Django URLconfs or a hand-wavy "don't worry, just do it").
I suggest that we deprecate this argument. This will require type-checking
the first argument to `patterns` for the duration of the deprecation
period, but I don't think that's so bad.
(I would not be opposed to deprecating the entire views-as-strings
feature, but the additional gain there is only code we can remove and stop
maintaining, not a benefit to the public API, so I decided to keep this
proposal minimal. If there's consensus to deprecate the entire feature, I
will happily update the summary to reflect that.)
--
Ticket URL: <https://code.djangoproject.com/ticket/22218>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by loic84):
* cc: loic@… (added)
* version: 1.6 => master
* stage: Unreviewed => Accepted
Comment:
+1 for this cleanup, I always considered `prefix` an anti-pattern.
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:1>
Django
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mjtamlyn):
Agreed we should get rid of prefix. I've got some ideas around which
actually involve changing URLconfs and resolving/reversing more
completely. In particular, if we deprecate `prefix` on patterns, then at
least 50% of the use case of `url()` rather than `RegexURLResolver` is
unneeded.
*<Going a bit off the real topic of this ticket now, but it's kinda
related>*
On the topic of beginners, I've heard a number of complaints about the
fact the only built in method of producing urls is regexes. Whilst these
are very powerful, learning the (somewhat obscure) syntax of
`(?P<poll_id>\d+)` and remembering it every time you want to write a URL
is a non-trivial hurdle (and that's before you start on the differing
behaviour of `\w` on Py2/3). Perhaps we should include a solution similar
to https://github.com/oinopion/hurl
Another irritation of mine is `django.conf.urls` vs
`django.core.urlresolvers` - everything should live in `django.urls`.
Also, the syntax for `django.shortcuts.reverse` is much better than that
of `django.core.urlresolvers.reverse` - the `kwargs={}` pattern is
annoying as (generally) you don't use args in a urlpattern.
Other related issues (which may or may not be resolved) are:
- The sea of `handlerXXX` functions (and being able to customise these
dependent on the patterns - API views vs real views)
- The general weirdness of namespacing (currently a powerful solution but
90% of use cases don't need the complexity)
- The ability to decorate views at the patterns level (rather than each
view individually)
- Deprecate models.permalink
- Our "absolute urls" are not absolute
- Reversing by dotted path to view is just a bad idea
I haven't yet decided what the new API should look like, but a significant
advantage of putting it in `django.urls` is all the main public functions
can stay exactly as they are for deprecation and proxy to the new version.
People using `RegexURLResolver` directly might have issues, but that can
likely be worked around as well. This probably needs a django-dev
discussion.
For the sake of clarity, I'm not against fixing this ticket on its own, I
think there's some value in it. However I would like (at some point) to
"fix" url resolving in general. This whole section of the code base (and
its design) are very old, and it might be worth reworking the whole thing,
rather than adding type checking on first arguments for a while.
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:2>
Django
Reporter: carljm | Owner: nobody
Type: Cleanup/optimization | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* cc: charettes (added)
Comment:
What about deprecating `django.conf.urls.patterns` altogether? If we
remove support for the `prefix` argument I see no compelling reason of
keeping it around.
Updating the documentation to use a list of `url`s instead should be
straightforward.
{{{#!python
from myapp import views
urlpatterns = [
url('^$', views.myview),
url('^other/$', views.otherview),
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:3>
Django
Reporter: carljm | Owner: timo
Type: Cleanup/optimization | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* status: new => assigned
* owner: nobody => timo
Comment:
+1 to deprecating `patterns` entirely. I've
[https://github.com/django/django/pull/2504 started working on this]. The
other thing that `patterns` provides besides the `prefix` parameter is
automatically wrapping plain tuples in `urlpatterns` in `url()`. I think
removing this and forcing usage of `url()` will be a good cleanup.
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:4>
Django
Reporter: carljm | Owner: timo
Type: Cleanup/optimization | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
Any thoughts about what we should do about
`django.conf.urls.i18n.i18n_patterns`?
I don't use it, but it looks like deprecating its call to `patterns` might
work.
{{{
def i18n_patterns(prefix, *args):
"""
Adds the language code prefix to every URL pattern within this
function. This may only be used in the root URLconf, not in an
included
URLconf.
"""
pattern_list = patterns(prefix, *args)
if not settings.USE_I18N:
return pattern_list
return [LocaleRegexURLResolver(pattern_list)]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:5>
Django
Reporter: carljm | Owner: timo
Type: Cleanup/optimization | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* has_patch: 0 => 1
Comment:
I will answer my own question and say we should deprecate just the
`prefix` argument to `i18n_patterns()`.
[https://github.com/django/django/pull/2504 PR] is ready for final review.
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:6>
Django
Reporter: carljm | Owner: timo
Type: Cleanup/optimization | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Gwildor):
This really cleans up the code a lot, wow. Awesome change!
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:7>
Django
Reporter: carljm | Owner: timo
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d73d0e071c1b4c86d57994a0ab55a74cfe80cdf5"]:
{{{
#!CommitTicketReference repository=""
revision="d73d0e071c1b4c86d57994a0ab55a74cfe80cdf5"
Fixed #22218 -- Deprecated django.conf.urls.patterns.
Thanks Carl Meyer for the suggestion and Alex Gaynor and Carl for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:8>
Django
Reporter: carljm | Owner: timo
Type: Cleanup/optimization | Status: closed
Component: Core (URLs) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e4708385fde278fbbff5428f756632455c2bcaf5"]:
{{{
#!CommitTicketReference repository=""
revision="e4708385fde278fbbff5428f756632455c2bcaf5"
Merge pull request #2825 from collinanderson/patch-2
Made url syntax consistent in tutorial; refs #22218.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:9>
Django
Reporter: carljm | Owner: timo
Type: Cleanup/optimization | Status: closed
Component: Core (URLs) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"a25d3ce007b90a0516aed54fc1c5a16510a290e4" a25d3ce0]:
{{{
#!CommitTicketReference repository=""
revision="a25d3ce007b90a0516aed54fc1c5a16510a290e4"
Refs #22218 -- Removed conf.urls.patterns() per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22218#comment:10>