Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#23112 closed Bug (fixed)

Field.get_choices tries to index an iterable: itertools.tee' object has no attribute '__getitem__'

Reported by: bernie_sumption Owned by: areski
Component: Database layer (models, ORM) Version: 1.7-rc-2
Severity: Release blocker Keywords:
Cc: apollo13, areski@…, pdewacht@…, lemuelf@…, cmawebsite@…, Jcuotpc@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Issue #23098 was fixed with this commit: https://github.com/django/django/commit/2f73b527dda6683868fac2791f7f07ccb01ea0d9

Which adds the following line:

named_groups = self.choices and isinstance(self.choices[0][1], (list, tuple))

According to the docs - https://docs.djangoproject.com/en/dev/ref/models/fields/#choices - Field.choices can be any iterable of choices. The above line introduces the requirement that the choices object to be subscriptable, and breaks both my own code and a 3rd party library I'm using (django_countries).

Change History (23)

comment:1 follow-up: Changed 8 months ago by magopian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Also, we can have a list of choices made of both named groups and "normal" choices, so we might no "see" that the blank choice is defined in the list of choices (it would be odd to not have it at the beginning of the list, but i don't think it's required, at least from the previous code and the docs).

comment:2 Changed 8 months ago by timo

  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from master to 1.7-rc-2

comment:3 in reply to: ↑ 1 Changed 8 months ago by apollo13

Replying to magopian:

Also, we can have a list of choices made of both named groups and "normal" choices, so we might no "see" that the blank choice is defined in the list of choices (it would be odd to not have it at the beginning of the list, but i don't think it's required, at least from the previous code and the docs).

That didn't work either previously, so we are somewhat safe in that regard… The original iterable argument seems valid, we can just call list on it I guess.

comment:4 Changed 8 months ago by apollo13

  • Cc apollo13 added

comment:5 Changed 8 months ago by areski

  • Cc areski@… added

I could reproduce the issue using an iterator, here a PR that fix the issue at my end
https://github.com/django/django/pull/2990

Thanks, I used list as @apollo13 mentioned.

Last edited 8 months ago by areski (previous) (diff)

comment:6 Changed 8 months ago by areski

  • Has patch set

The PR now contains tests: https://github.com/django/django/pull/2990
I noticed that with Iterators, we cannot use _get_FIELD_display properly, now this problem might not be blocking.

comment:7 Changed 8 months ago by zsiciarz

I got bitten by this bug too (talked about it yesterday with Baptiste). Here's a traceback from my project which used to work on RC1, breaks on RC2.

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/core/management/__init__.py", line 354, in execute
    django.setup()
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/__init__.py", line 21, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/apps/registry.py", line 115, in populate
    app_config.ready()
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/contrib/admin/apps.py", line 22, in ready
    self.module.autodiscover()
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/contrib/admin/__init__.py", line 23, in autodiscover
    autodiscover_modules('admin', register_to=site)
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/utils/module_loading.py", line 74, in autodiscover_modules
    import_module('%s.%s' % (app_config.name, module_to_search))
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/importlib/__init__.py", line 90, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1584, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1565, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1532, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 584, in _check_name_wrapper
  File "<frozen importlib._bootstrap>", line 1022, in load_module
  File "<frozen importlib._bootstrap>", line 1003, in load_module
  File "<frozen importlib._bootstrap>", line 560, in module_for_loader_wrapper
  File "<frozen importlib._bootstrap>", line 868, in _load_module
  File "<frozen importlib._bootstrap>", line 313, in _call_with_frames_removed
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/allauth/socialaccount/admin.py", line 12, in <module>
    class SocialAppForm(forms.ModelForm):
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/forms/models.py", line 284, in __new__
    opts.help_texts, opts.error_messages)
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/forms/models.py", line 210, in fields_for_model
    formfield = f.formfield(**kwargs)
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/db/models/fields/__init__.py", line 1063, in formfield
    return super(CharField, self).formfield(**defaults)
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/db/models/fields/__init__.py", line 822, in formfield
    defaults['choices'] = self.get_choices(include_blank=include_blank)
  File "/home/zbigniewsiciarz/v/bazawiedzy/lib/python3.3/site-packages/django/db/models/fields/__init__.py", line 733, in get_choices
    named_groups = self.choices and isinstance(self.choices[0][1], (list, tuple))
TypeError: 'itertools._tee' object is not subscriptable

comment:8 Changed 8 months ago by areski

Did you try with this patch https://github.com/django/django/pull/2990?

comment:9 Changed 8 months ago by pdewacht

  • Cc pdewacht@… added

comment:10 Changed 8 months ago by zsiciarz

Yes, wrapping self.choices with a list() call fixed the problem.

comment:11 Changed 8 months ago by areski

Update of the PR thanks to the feedback from @Apollo13
https://github.com/django/django/pull/2990

comment:12 Changed 8 months ago by areski

  • Owner changed from nobody to areski
  • Status changed from new to assigned

comment:13 Changed 8 months ago by areski

Following @timgraham advice, the commented tests has been removed from the PR

comment:14 Changed 8 months ago by lemuelf

  • Cc lemuelf@… added

comment:15 Changed 8 months ago by CollinAnderson

  • Cc cmawebsite@… added

comment:16 Changed 8 months ago by CollinAnderson

As noticed in #23119, the patch doesn't work if the iterable is empty (empty iterable is fine on 1.6)

field = models.CharField(choices=(x for x in []), blank=True, max_length=1)

raises IndexError: list index out of range with the pull request. No problem on 1.6

comment:17 Changed 8 months ago by CollinAnderson

  • Summary changed from Field.get_choices tries to index an iterable to Field.get_choices tries to index an iterable: itertools.tee' object has no attribute '__getitem__'

comment:18 Changed 8 months ago by areski

Thanks @CollinAnderson check the pull request, I just added a fix for empty iterators and also some regression tests

comment:19 Changed 8 months ago by jcuotpc

  • Cc Jcuotpc@… added

comment:20 Changed 8 months ago by jcuotpc

  • Cc Jcuotpc@… added

comment:21 Changed 8 months ago by CollinAnderson

The pull request works for me. I closed #23119 as duplicate.

comment:22 Changed 8 months ago by Florian Apolloner <florian@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 97a38de230371c0b6ad8a86abba8425186c147c7:

Fixed #23112 -- Field.get_choices tries to index an iterable

comment:23 Changed 8 months ago by Florian Apolloner <florian@…>

In e22ad1c3250a59d046d73bf34ae39c0c40e03c98:

[1.7.x] Fixed #23112 -- Field.get_choices tries to index an iterable

Backport of 97a38de230371c0b6ad8a86abba8425186c147c7 from master.

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