Opened 6 years ago
Last modified 6 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 , 6 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 , 6 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 , 6 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 , 6 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 , 6 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
versionchangedannotation.