django-rest-framework-gis

[jayvdb]

https://github.com/openwisp/django-rest-framework-gis causes warning

could not resolve serializer field GeometryField(allow_null=True, required=False). defaulting to "string"

It definitely isnt a string ;-)

[tfranzel]

i'd like to see 3rd party support for GIS. i stumbled on it before. would you mind doing a PR for it to spread the workload a bit?

before you can do it, i need to introduce another extension mechanic. We already have OpenApiSerializerExtension but this also requires OpenApiSerializerFieldExtension which i will add. this is the second time i would have needed it.

[jayvdb]

Ya, I am relying on these fields so I need it sooner than later, so I'll do the final part of getting it working and tested, after you have done any helpers you think will help.

[tfranzel]

the facilities are now available. have a look at https://github.com/tfranzel/drf-spectacular/blob/master/tests/test_extensions.py on how to use it. this should allow you to address pretty much every custom serializer field.

the extensions, as mentioned before, can be located anywhere as long as the interpreter comes by at least once. please do a PR once you are happy with the GIS extensions. would be much appreciated.

[jayvdb]

Created draft #38 . I havent played with it yet, but it gives you an idea of what else we might want to support custom complex fields.

The most obvious is that the schema is not small, and it isnt ideal for it to be inline. I have opt for compressing lots of the geo types into one GEOS_GEOMETRY to keep it small.

Ideally there is some way for the class to expose extra definitions that can be accessed using $ref.

Alternatively it might be possible to use external schema - I havent checked whether drf-spectacular supports that, and I personally also want to ensure that the validator supports that. When I tried to use an external schema, the field was automatically given readOnly: true which I do not want. I didnt look into whether that was fixable.

[MissiaL]

It seems to me that using some other serializers from other packages is not a good idea. You must use the logic of constructing the schema in your project using drf-spectacular

[tfranzel]

@jayvdb that looks pretty nice already. you can relatively easily use the ref system:

drf-spectacular/drf_spectacular/openapi.py

Lines 816 to 820 in 833f984

	component = ResolvedComponent(
	name=self._get_serializer_name(serializer, direction),
	type=ResolvedComponent.SCHEMA,
	object=serializer,
	)

adapt for your case.

coponent.schema = YOURGISSCHEMA
auto_schema.registry.register(component)
return component.ref

i don't like the external schema idea. i makes sense but i'm pretty sure code generators will choke on that.

@MissiaL could you elaborate why it is not a good idea? there are 3rd party apps that can throw a lot of warnings and their code is not reachable for adaptation if you just use them. in those cases you cannot use @extend_schema and would be forced to override every lib serializer/field with annotation (if even possible).

furthermore, i think it is not a bad idea to support very commonly used packages. the plugin system is specifically designed to have them in contrib so they do not interfere with the core logic.

[MissiaL]

Maybe I'm wrong here. I agree with you. Support for popular packages is very good 😊

[jayvdb]

Great. Thanks for the hints about the component registry; that makes the implementation a lot more clean.

Ideally I would like to store the GeoJSON schema in a file in the package (or finding a canonical/stable versioned copy online), and then loading the file on demand and registering the necessary parts of the schema. That has the benefit of the schema being more accessible/interoperable with other tools, and avoid hidden bugs created in a hand-crafted Pythonised version of the GeoJSON schema.

There are generic JSON fields in Django (and will be cross dbms in Django soon! https://twitter.com/laymonage/status/1242993372819640320), where the app logic decides what data to stuff into them, so some user-friendly mechanism to map a Field(and a field instance) to an external spec will be sorely needed. Due to the problems of external schema & references, "internalising" those external schemas seems to be the way to have your cake and eat it.

[tfranzel]

i'm not convinced yet to bloat the package with the GeoJSON schema, but i absolutely get your point. Can we do it without on the first try? if it really makes sense we could still add it later. is that reasonable? not sure yet how to strike a good balance between spectacular being convenient to use and going down the emacs route.

regarding the generic json. that sounds awesome! i had to simulate that on several occations and it would be really good for schemas. once its lands in DRF we definitely going to support that.

[jayvdb]

Well, we would have the GeoJSON schema either way - either as hand crafted Python objects as per my PR (and my current schema there is very dodgy due to jumbling lots of geo types into a single loose schema using anyOf), or as an external file which we load into Python objects.

I'm happy to continue with the current approach atm, and we can come back to this later when more JSON fields need to be supported. https://www.django-rest-framework.org/api-guide/fields/#jsonfield is already part of DRF. It has historically only been supported with core PostgreSQL django.contrib.postgres.JSONField, but https://pypi.org/search/?q=drf+jsonfield has lots of workarounds people have been using, so you can expect many users are going to be hitting this problem with adoption.

[jayvdb]

We should also test django-geoposition , and integrate it into the existing PR. There is also another common one in a package named something like django-extra-fields.
(btw, I'll be getting to this in about a week; a few other fires to put out first)

[christiaan-dockbite]

This feature would be very useful

[sayeghr]

Love the library, just want to +1 the sentiment that this feature would be very useful.

[SidneyNiccolson]

Also would love to have this feature (+1)!

[AJ-BM]

I would also love support for Djangos GeometryField

[tfranzel]

thank you very much for the feedback and the appreciation!

I myself am not a user of django-rest-framework-gis. Due to time constraints I am focusing on fixing issues in the core system at the moment. I simply lack the time to implement this or negotiate a solution with upstream.

It is possible that openwisp/django-rest-framework-gis/pull/223 gives us the required facilities, but I have not studied it in detail. @jayvdb may chime in on this and comment on whether there are any road blocks left. If anyone is attempting to do this, make sure you also read up on #38.

[christiaan-dockbite]

@tfranzel I am confused why #38 cannot be merged. I would be very happy with the feature, so I am happy to contribute to it. I do not know however where to start. Can you give some explanation on this?

[tfranzel]

hey @christiaan-dockbite

• WIP: Add drf-gis fields #38 : outdated, broken and likely incomplete (author @jayvdb)
• GIS fixes for #38 #67 : approach taken from 38, updated, extended, cleaned up. might still be incomplete (author myself)

I stopped working on it because it received little feedback and most importantly I did not want to cannibalize @jayvdb efforts. #67 served merely as basis for discussion. i just rebased it from time to time, which I just did again. @jayvdb offered to pick it up again, but gave no signals since then. i guess it's up for grabs.

Then openwisp/django-rest-framework-gis/pull/223 updated their schema support, which is quite frankly a ridiculous upstream decision. The gist is that every lib is supposed to write their own AutoSchema. Not quite sure how that was ever supposed to work with more than 1 non-trivial library, but i digress.

The remaining question is basically this:
Do we bother extracting their schema from openwisp/django-rest-framework-gis/pull/223 or is our generic approach sufficient?We basically took the GeoJSON definition and massaged it a bit. I have no idea if django-rest-framework-gis completely adheres to the GeoJSON standard.

[Christiaanvv]

@tfranzel see #532

[ThalusA]

This is still not fix ?

[tfranzel]

finally resolved by #746.

The new approach is forked from django-rest-framework-gis but adapted to our needs including some bugfixes. The reasoning behind this is that the AutoSchema subclassing approach was rendered obsolete by DRF's maintainers.