Skip to content

Conversation

iGeophysix
Copy link
Contributor

As per documentation you can define gis_widget_kwargs in GISModelAdmin as
"The keyword arguments that would be passed to the gis_widget. Defaults to an empty dictionary."
https://docs.djangoproject.com/en/4.0/ref/contrib/gis/admin/#django.contrib.gis.admin.GISModelAdmin

But it takes attrs as dict.
Fixed it to take kwargs as new attrs

As per documentation you can define gis_widget_kwargs in GISModelAdmin as
"The keyword arguments that would be passed to the gis_widget. Defaults to an empty dictionary."
https://docs.djangoproject.com/en/4.0/ref/contrib/gis/admin/#django.contrib.gis.admin.GISModelAdmin

But it takes attrs as dict.
Fixed it to take kwargs as new attrs
@iGeophysix
Copy link
Contributor Author

or is it better to replace line 21 : self.gis_widget(**self.gis_widget_kwargs) with self.gis_widget(attrs=self.gis_widget_kwargs) in
/django/contrib/gis/admin/options.py

@github-actions
Copy link

Hello @iGeophysix! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

@ngnpope
Copy link
Member

ngnpope commented Dec 16, 2021

Hi @iGeophysix.

Thanks for bringing this to our attention. I've opened ticket-33372 - you can log in there and assign the ticket to yourself.

or is it better to replace line 21 : self.gis_widget(**self.gis_widget_kwargs) with self.gis_widget(attrs=self.gis_widget_kwargs) in /django/contrib/gis/admin/options.py

OSMWidget inherits from Widget which has the signature (self, attrs=None) as the signature for .__init__().

So, yes, please go with this alternate solution you have mentioned. Please can you also add some tests as there clearly isn't any coverage for this.

It is a shame that gis_widget_kwargs wasn't called gis_widget_attrs.
@felixxm Do you think we can rename this given that it doesn't work properly anyway? We'd just need to highlight it carefully in the release notes.

/cc @claudep

@iGeophysix iGeophysix closed this Dec 17, 2021
@iGeophysix iGeophysix deleted the patch-1 branch December 17, 2021 05:10
@iGeophysix iGeophysix restored the patch-1 branch December 17, 2021 05:17
@iGeophysix
Copy link
Contributor Author

@ngnpope thank you for your comments.
I'll create a new pull request as described above with tests and will wait for your decision regarding renaming the attribute to gis_widget_attrs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants