Opened 2 months ago

Closed 2 months ago

Last modified 8 weeks ago

#35766 closed Bug (fixed)

Choice iterator breaks when using slices

Reported by: David Owned by: Sarah Boyce
Component: Database layer (models, ORM) Version: 5.0
Severity: Normal Keywords:
Cc: Natalia Bidart, Nick Pope Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by David)

Currently the choice-iterator classes introduced in https://code.djangoproject.com/ticket/24561 are designed to work with index-based access.

   

class MyModel(models.Model):
  field = models.IntegerField(choices=lambda: range(10))


the_field = MyModel._meta.get_field("field")
the_field.choices[2]
#> 3

Since choices has been out for long time accepting there are libraries in which the field.choices attribute was used as it it was a tuple/list, which can be accessed also with slicing syntax (see sphinxcontrib-django ), this now raises an error:

the_field.choices[:2]
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[13], line 1
----> 1 the_field.choices[:2]

File /opt/venv/lib/python3.10/site-packages/django/utils/choices.py:24, in BaseChoiceIterator.__getitem__(self, index)
     23 def __getitem__(self, index):
---> 24     if index < 0:
     25         # Suboptimally consume whole iterator to handle negative index.
     26         return list(self)[index]
     27     try:
TypeError: '<' not supported between instances of 'slice' and 'int'

The __getitem__ states that the management of key type should be handled in the implementation.

It should be choosen if slices are going to be supported or if only integers are going to be supported by this class.

Change History (10)

comment:1 by David, 2 months ago

Description: modified (diff)

comment:2 by Sarah Boyce, 2 months ago

Cc: Natalia Bidart Nick Pope added
Component: UncategorizedDatabase layer (models, ORM)
Has patch: set
Owner: set to Sarah Boyce
Status: newassigned
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Thank you, replicated
I imagine we do want to support it (but I might be wrong) - made a small patch

comment:3 by Sarah Boyce, 2 months ago

Triage Stage: AcceptedReady for checkin

PR received approval

comment:4 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

Resolution: fixed
Status: assignedclosed

In ae1ee241:

Fixed #35766 -- Handled slices in BaseChoiceIterator.

comment:5 by Claude Paroz, 2 months ago

Sarah, isn't this a regression in the last Django version that warrants a backport?

in reply to:  5 comment:6 by Natalia Bidart, 8 weeks ago

Replying to Claude Paroz:

Sarah, isn't this a regression in the last Django version that warrants a backport?

Hey Claude, thanks for keeping an eye on things!
But the feature was introduced in 5.0 so it does not qualify for a backport at this time.

comment:7 by Claude Paroz, 8 weeks ago

Isn't the related commit 07fa79ef2bb3e8cace7bd87b292c6c85230eed05 ?

comment:8 by Sarah Boyce, 8 weeks ago

Yes you're right - I missed this. I will create a release note a back port shortly

comment:9 by Sarah Boyce, 8 weeks ago

PR - have some questions as to whether it is required for the backport

comment:10 by Natalia Bidart, 8 weeks ago

I still think that this does not qualify for a backport: 07fa79ef2bb3e8cace7bd87b292c6c85230eed05 was landed in main right before the 5.0 beta release, that means during the 5.0 alpha-beta phase. Thus, it was backported to the stable/5.0.x branch due to the "alpha" backporting policy. Therefore, this commit is really a "5.0 feature" and it's my understanding that this does not qualify for a 5.1 backport.

Note: See TracTickets for help on using tickets.
Back to Top