Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix grouped choices as dict #1668

Merged
merged 4 commits into from
Aug 2, 2024
Merged

Conversation

saevarom
Copy link
Contributor

Here is an attempt to fix #1667

I decided to go for a simple approach in normalizing the choices, and used a utility function from django 5.0. I think that's OK since this method of defining choices was first introduced in django 5.0.

Copy link
Owner

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thanks.

@carltongibson
Copy link
Owner

Oh, except the tests didn't like it...

@carltongibson
Copy link
Owner

@saevarom I'm looking towards a release. Do you have time to address the failing test on Django 4.2?

@saevarom
Copy link
Contributor Author

I'll take a look now

@saevarom
Copy link
Contributor Author

@carltongibson I've pushed a fix, could you approve the test run?

@carltongibson
Copy link
Owner

@saevarom great thanks. The isort run is playing up. I can look at that later on. Other than that 👍

@carltongibson
Copy link
Owner

django_filters/filters.py:34:5: F401 'django.utils.choices.BaseChoiceIterator' imported but unused

@carltongibson
Copy link
Owner

Great. Thanks 🎁

Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  django_filters
  filters.py 494
Project Total  

This report was generated by python-coverage-comment-action

@carltongibson
Copy link
Owner

@saevarom If we want perfect coverage here we'd also need to check the assert is raised for Django 4.2...

#1668 (comment)

@carltongibson carltongibson merged commit 23be051 into carltongibson:main Aug 2, 2024
9 checks passed
@carltongibson
Copy link
Owner

This is now available in v24.3 on PyPI. Thanks @saevarom!

last-partizan pushed a commit to last-partizan/django-filter that referenced this pull request Sep 12, 2024
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.

Support group mappings as dictionares introduced in Django 5.0
2 participants