Some concern about the implementation of custom field types

[pmdevita]

With the merging of #1352, a long standing feature request for support for custom field types has now been solved. However, after thinking about it for a bit, I think this solution has a few problems.

1. Global variables

The global list for type mapping was simple, but I think that's fine. However, having a function that simply edits that list is a "code smell", I know Django relies on globals a lot but this feels like a step too far, and it has some knock on effects.

2. Load order

Because a function must be called to register a type, that function now must be called before the schema using it is initialized. I suppose since models already must be loaded first this might always be OK, but then the registration call has to live somewhere that gets loaded first. This is really a piece of configuration, so maybe it should live in settings.py? But it's a function call, not a list or dictionary...

3. Inheritance

I think this is the biggest thing the current solution lacks. There are a number of libraries that implement custom fields that are just subclassing built-in Django fields. The current solution requires you to register each of these manually, but it would be a lot nicer if Ninja could determine this automatically. For fully custom fields that also have subclasses, these all must be manually registered too.

Additionally, because it's a function call, library authors who may wish to add support for Ninja to their library have a more difficult task. They cannot simply import that function and call it, they need to check if Ninja is an available library and they probably should also check if it's being loaded in the Django project.

With that all in mind, I think this solution #1320 had some advantages in this regard. It avoids global variables and load order issues, and provides an avenue for inheritance and library support. I would like to see some kind of hybrid solution, one that can either pull the type from the special function on the field, or introspect its class and pull a type from the mapping of built-in fields.

[vitalik]

I observed for a while how people were just using TYPES global variable and it worked pretty fine..

indeed you have to "register" it somewhere before you import your model-schemas, but overall that looks easy

the problem with FIeld.get_schema_type - is that other library authors have no intention implementing it, so without register global user will have to inherit custom field and create get_schema_type - which would be annoying experience

[pmdevita]

the problem with FIeld.get_schema_type - is that other library authors have no intention implementing it, so without register global user will have to inherit custom field and create get_schema_type - which would be annoying experience

I don't think that's necessarily true, I think we're seeing Ninja emerge as the leading choice for new API projects on Django, and it's definitely not the only project that would benefit from better integration with Pydantic. But I agree, large-scale adoption of the practice would be a long time away if it even ever happened, which is why I think some kind of hybrid approach is necessary.

As I'm writing, I now think maybe it would be possible to achieve this in settings somehow. Maybe a dictionary mapping import paths to a Pydantic type? If we're going to keep this paradigm, it's really a setting and should probably live in settings.py.

For example, with something like VectorField from pgvector,

NINJA_CUSTOM_TYPES = {
    "pgvector.django.vector.VectorField": List[int]
}

This approach feels a lot more "Django" to me.

pgvector is also a great example of something that would benefit from the function though, as one would probably want to validate its dimensions and that's going to be dependent on the field's settings.

class VectorField(Field):
    ...

    def get_schema_type(self):
        from pydantic import conlist
        return conlist(int, min_length=self.dimensions, max_length=self.dimensions)

This constraint is a more accurate adaptation, but without the function, you would have to manually define this field on every schema that uses it.

[vitalik]

NINJA_CUSTOM_TYPES = {
    "pgvector.django.vector.VectorField": List[int]
}

Well I think this one is the least "comfortable" as it fully need user attention to configure it in the project

register_field gives library creators a way to integrate with django ninja without forcing users to configure anything (if they want to or care at all)

in fact some may even create a pip package that will collect most popular django packages which provide custom fields and register it al at once based on what's installed in system packages

get_schema_type on the other hand breaks few SOLID principles (like why would ever pgvector developers add that method - spoiling the code base for users who may not even use django ninja)