GISModelAdmin.gis_widget_kwargs cannot be used due to signature mismatch.

[Nick Pope]

This issue was raised in ​https://github.com/django/django/pull/15207.

The problem is that the gis_widget_kwargs attribute of GeoModelAdminMixin is **-unpacked into gis_widget which is OSMWidget by default.

OSMWidget and BaseGeometryWidget have .__init__(self, attrs=None), not .__init__(self, **kwargs), so this is broken.

Marking as a release blocker as this is a bug in a new feature introduced in 4.0.

[Mariusz Felisiak]

Replying to Nick Pope:

OSMWidget and BaseGeometryWidget have .__init__(self, attrs=None), not .__init__(self, **kwargs), so this is broken.

Marking as a release blocker as this is a bug in a new feature introduced in 4.0.

I wouldn't say it's broken, you can set gis_widget_kwargs = {'attrs': {...}}. Defining widgets with more than attrs or a different signature is reasonable (e.g. SplitDateTimeWidget or ChoiceWidget). IMO we should leave wide support for all options.

[Nick Pope]

Replying to Mariusz Felisiak:

I wouldn't say it's broken, you can set gis_widget_kwargs = {'attrs': {...}}. Defining widgets with more than attrs or a different signature is reasonable (e.g. SplitDateTimeWidget or ChoiceWidget). IMO we should leave wide support for all options.

I see. That makes sense, but I can also see that it is confusing.
Perhaps the documentation can be updated with an example? (If only to avoid reports of this in the future.)
In addition, this is not tested - it should be to ensure that gis_widget_kwargs continues to work as expected.

[Mariusz Felisiak]

Replying to Nick Pope:

Perhaps the documentation can be updated with an example? (If only to avoid reports of this in the future.)

Sounds reasonable. Maybe a short example with changing default_lon and default_lat.

In addition, this is not tested - it should be to ensure that gis_widget_kwargs continues to work as expected.

Extra tests coverage is always warm welcome.

[Alexander Filimonov]

Replying to Nick Pope:

Replying to Mariusz Felisiak:

I wouldn't say it's broken, you can set gis_widget_kwargs = {'attrs': {...}}. Defining widgets with more than attrs or a different signature is reasonable (e.g. SplitDateTimeWidget or ChoiceWidget). IMO we should leave wide support for all options.

I see. That makes sense, but I can also see that it is confusing.
Perhaps the documentation can be updated with an example? (If only to avoid reports of this in the future.)
In addition, this is not tested - it should be to ensure that gis_widget_kwargs continues to work as expected.

I'd support Nick here and say that it is not a good way to use a dictionary to fill a single attribute.
I have a fix for this with a test to check if arguments are passed correctly
​https://github.com/django/django/pull/15211
Please review the pull request

[Mariusz Felisiak]

I'd support Nick here and say that it is not a good way to use a dictionary to fill a single attribute.

As far as I'm aware Nick agreed that the current implementation makes sense. Of course, it's worth adding test coverage and a short example in docs.

[Alexander Filimonov]

Please have a look at the pr - little fix will make it more obvious :-)

[Mariusz Felisiak]

Please don't reopen closed ticket. The fix is not backward compatible and it introduces unnecessary limitation.