Change `to_python` and `to_url` to be class methods on path converters

[Petter friberg]

Hi,

I ran in to the situation of wanting `to_python` and `to_url` be decorated with `@classmethod` to avoid having to declare an additional (kind of no-op) class with those 2 methods.

Given that I have a couple of custom python data structures that can be encoded into path params, it would be slightly more cleaner if my custom class(es) would be the ones declaring `def to_python` and `def to_url` to comply with the path converter interface. Rather than having some additional `class <MyType>Converter`.

Below follows an invented example, where I pretend to have some product slug composed of a slugged name and integer code. Imagine seeing something like “white-socks-123456789” in a URL.

```

class Slug:
    regex = r".+"

    def __init__(self, slugged_name, sku):
        self.slugged_name = slugged_name
        self.sku = sku

    @classmethod
    def to_python(cls, value):
        name, _, sku = value.rpartition("-")
        if not name:
            raise ValueError("invalid name segment")

        try:
            sku = int(sku)
        except ValueError as exc:
            raise ValueError("sku needs to be a number") from exc

        return cls(name, sku)

    @classmethod
    def to_url(cls, slug):
        return f"{slug.slugged_name}-{slug.sku}"


register_converter(Slug, "my-slug")

```

Could changing the converter interface be something to consider?

At least I couldn’t see any obvious problems with expecting these 2 methods as `@classmethod` (or even `@staticmethod`?) instead. But I could very well be missing something.

Thanks!

Petter

[Adam Johnson]

I can see the appeal of allowing you to have fewer classes, but I don't think that's a good thing in itself. Keeping responsibilities separate ("convert slugs to/from URL's" versus "be a slug") is a generally desirable property.

This change would also complicate the URL parsing process: it would need to check whether the methods are class or instance level, and instantiate the converter or not. We wouldn't want to force all converters to move to use classmethods as that would be a backwards incompatible change, requiring a deprecation cycle.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/a43092e4-34a1-403a-beef-a2c4cac710ebn%40googlegroups.com.

[Petter friberg]

Thank you for your response!

> [...] Keeping responsibilities separate ("convert slugs to/from URL's" versus "be a slug") is a generally desirable property. [...]

While I do agree with you, I'm kind of wondering if it's desirable that Django enforces how and when the responsibilities are kept separate. Couldn't that be something left for the developer to decide?

In my case it happens to almost look like copying rather than keeping responsibilities separate, i.e. the opposite of the DRY principle.

> This change would also complicate the URL parsing process: it would need to check whether the methods are class or instance level, and instantiate the converter or not. We wouldn't want to force all converters to move to use classmethods as that would be a backwards incompatible change, requiring a deprecation cycle.

I was kind of aware of that, but more of wondering if it you saw it worth to deprecate the non-classmethod interface in favor of one expecting classmethods. But I understand that it's barely any gain for the amount of work it involves.

---

In any case, I realised that it's actually currently possible to implement these 2 methods as classmethods, if I wrap the registration in a lambda/callable. e.g.

```

register_converter(lambda: Slug, "my-slug")

```

/Petter