-
-
Notifications
You must be signed in to change notification settings - Fork 7k
fix: MultipleChoiceField use ordered sort #9735
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
base: main
Are you sure you want to change the base?
Conversation
auvipy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also add unit tests to verify this and make the CI green
I’ve added the tests and the CI is green now. |
browniebroke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems sensible to me 👍🏻
However, it will have to wait for 3.17 as I see this as potentially disruptive for some downstream users. Left a small suggestion
Good point — waiting for 3.17 sounds like the right call. Appreciate the suggestion! |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modifies MultipleChoiceField to return ordered lists instead of sets, ensuring JSON serialization compatibility and maintaining the order of input values. The change uses dict.fromkeys() to deduplicate values while preserving insertion order.
Key Changes
- Changed
to_internal_value()andto_representation()methods to return lists usinglist(dict.fromkeys([...])) - Updated all test expectations from sets to lists
- Added JSON serialization tests to prevent regression
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rest_framework/fields.py | Modified MultipleChoiceField methods to return ordered lists instead of sets using dict.fromkeys() for deduplication |
| tests/test_fields.py | Updated test expectations from sets to lists, added order-preserving test case, and added JSON serialization validation tests |
| tests/test_serializer_nested.py | Updated nested serializer test assertions to expect lists instead of sets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This modification has the following advantages: