Keywords not supported in distill_path

henhuy commented 3 years ago

I stumbled upon keywords in distill_path function when switching from default django.urls.path to distill_path. My original path looked something like this:

path(
    "distilled_forest/<int:wind_distance>/<int:forest_usage>",
    view=views.DistilledForestListAPIView.as_view(),
    name="distilled_forest",
)

Simply changing it into:

distill_path(
    "distilled_forest/<int:wind_distance>/<int:forest_usage>",
    view=views.DistilledForestListAPIView.as_view(),
    name="distilled_forest",
    distill_func=models.Setup.get_possible_setups
)

gave me a strange error without stack trace:

Loading site URLs
CommandError: Invalid view arguments

But I could figure it out: distill_path expects first two arguments given as simple args (not kwargs) - See line: https://github.com/meeb/django-distill/blob/ea6132b6c21c73265c5b80389c9fb448c9eb7297/django_distill/renderer.py#L120 This works:

distill_path(
    "distilled_forest/<int:wind_distance>/<int:forest_usage>",
    views.DistilledForestListAPIView.as_view(),
    name="distilled_forest",
    distill_func=models.Setup.get_possible_setups
)

Maybe this could be changed to support keyword args - or at least a better Exception with explanation could be thrown?

meeb commented 3 years ago

Django's django.urls.conf.path uses positional not keyword args, the plan with distill would be to mirror what Django does internally.

https://github.com/django/django/blob/master/django/urls/conf.py#L57

Your view= kwarg is actually incorrect and should probably be changed to a positional arg. I'd reconsider this if Django itself made the change. As you've noted, all distill does is pop off the distill_func= and optionally distill_file= kwargs then just passes *args and **kwargs back to Django's normal path() function so distill itself doesn't interfere with how it should URL definitions should be working.

henhuy commented 3 years ago

You are right - my version works for django, but originally it is a simple arg. Most of the time, I use keywords in order to make things clearer at first sight. Sorry for bothering you...

meeb commented 3 years ago

No problem at all, thanks for the contributions.

meeb / django-distill

Keywords not supported in distill_path #27