Opened 5 years ago
Last modified 5 years ago
#31069 closed Bug
Change in behavior for optional re matches. — at Version 5
Reported by: | Terence Honles | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 3.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
I am mainly pointing this out as it probably should have been called out as a breaking change from Django 2.X to 3.X (And I think the release notes should probably be adjusted if the old functionality is not to be restored). You can probably argue either way about if this is a bug since it's coming from re's match.groupdict()
. It's related to https://code.djangoproject.com/ticket/31061 and caused by https://code.djangoproject.com/ticket/26431 / https://github.com/django/django/pull/11477
The test is the following:
from django.urls import re_path def view(request, optional1, optional2): ... urlpatterns = [ re_path(r'^(?P<optional1>\w+/)(?:(?P<optional2>\w+)/)?', view) ]
For Django 2.X this would work as expected because named captures are returned even if they are None
, however in Django 3.0 this would raise an error because positional parameter optional2
is no longer provided. (i.e. view() missing 1 required positional argument: 'optional2'
)
You can argue that optional2
should actually have had a default all along, but it didn't before and code may unexpectedly break because it is now required.
I'm not very familiar with https://code.djangoproject.com/ticket/26431, but I think if possible it probably should have been fixed at a different level.
Change History (5)
comment:1 by , 5 years ago
Component: | Core (URLs) → Documentation |
---|---|
Severity: | Normal → Release blocker |
Summary: | Change in behavior for optional re matches → Change in behavior for optional re matches. |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 5 years ago
I can't reproduce the view() missing 1 required positional argument: 'optional2'
error with the URLconf you've provided. There aren't any optional capture groups in your regexp so I don't see how groupdict()
would return None
values.
Do you have an example URL that triggers the error?
Thanks.
comment:3 by , 5 years ago
error is caused by missing ? at end of re_path
from django.urls import re_path def view(request, optional1, optional2): ... urlpatterns = [ re_path(r'^(?P<optional1>\w+/)(?:(?P<optional2>\w+)/)?', view) ]
works
comment:4 by , 5 years ago
Severity: | Release blocker → Normal |
---|
Issue that Terence described is that we no longer pass None
for named groups that are not provided. The following example
def modules(request, format): return render(request, 'home.html') urlpatterns = [ path('admin/', admin.site.urls), re_path(r'^module1/(?P<format>(html|json|xml))?/?$', modules, name='modules1'), ]
works in Django 2.2 when we call http://localhost:8000/module1/
but throws an error in Django 3.0:
response = wrapped_callback(request, *callback_args, **callback_kwargs) TypeError: modules() missing 1 required positional argument: 'format'
IMO this behavior is correct, see comment.
I'm decreasing a severity because it's a documentation issue and can be backported at anytime.
comment:5 by , 5 years ago
Description: | modified (diff) |
---|
I think that the current behavior is correct, sorry for inconvenience. We should clarified this in documentation, add release notes and a
versionchanged
annotation.