Django

Code

Ticket #5445 (closed: fixed)

Opened 10 months ago

Last modified 10 months ago

Checking iterables by looking for a __iter__ attribute doesn't work on Jython

Reported by: leosoto <leo.soto@gmail.com> Assigned to: nobody
Milestone: Component: Core framework
Version: SVN Keywords: jython, sprintsept14
Cc: Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

Jython list and tuples are iterables but don't have iter, so are incorrectly detected as non-iterables by model validation.

Attachments

iterables.patch (3.4 kB) - added by leosoto <leo.soto@gmail.com> on 09/14/07 01:26:55.
Replaced iter attribute lookup by trial an error with iter()
iterables.2.patch (3.4 kB) - added by leosoto <leo.soto@gmail.com> on 09/14/07 01:29:24.
added a missing header on the diff file
iterables2.patch (3.5 kB) - added by leosoto <leo.soto@gmail.com> on 09/14/07 11:22:51.
another patch following malcom's suggestions
iterables3.patch (4.6 kB) - added by leosoto <leo.soto@gmail.com> on 09/14/07 14:03:36.
Uses is_iterable on django.template too. Also changes the order of the typical check for the common case: now we first check for non-strings, next for iterables.
iterables3.2.patch (4.5 kB) - added by leosoto <leo.soto@gmail.com> on 09/14/07 14:11:41.
The previous patch file is messed up. Now it's OK.

Change History

09/14/07 01:26:55 changed by leosoto <leo.soto@gmail.com>

  • attachment iterables.patch added.

Replaced iter attribute lookup by trial an error with iter()

09/14/07 01:29:24 changed by leosoto <leo.soto@gmail.com>

  • attachment iterables.2.patch added.

added a missing header on the diff file

09/14/07 01:40:08 changed by Simon G. <dev@simon.net.nz>

  • needs_better_patch changed.
  • stage changed from Unreviewed to Ready for checkin.
  • needs_tests changed.
  • needs_docs changed.

09/14/07 02:05:53 changed by mtredinnick

  • stage changed from Ready for checkin to Accepted.

I'm not in love with the implementation. A couple of comments:

  • you create _is_iterable in validation.py and then later on do almost the exact same thing in test/client.py. Why not create a file in django/utils/ called something appropriate (or use itercompat.py, which we already have) and put a utility function in there?
  • Does Jython have a basestring type? We use isinstance(foo, basestring) all over the place in Django, so it'd be nice we could also use that here instead of types.StringType?. Otherwise there are a lot of places you're going to have to change.

09/14/07 11:22:51 changed by leosoto <leo.soto@gmail.com>

  • attachment iterables2.patch added.

another patch following malcom's suggestions

09/14/07 13:25:22 changed by Fredrik Lundh <fredrik@pythonware.com>

Looks reasonable. Not sure about the name "itercompat", though. CPython does not guarantee that all iterables have an iter method; they may just implement the tp_iter slot on the C level, or just provide a C-level sequence API. iter() handles all those cases.

09/14/07 13:31:44 changed by Fredrik Lundh <fredrik@pythonware.com>

(oh, sorry, missed that itercompat.py is an existing module. +1 from me, then)

09/14/07 14:03:36 changed by leosoto <leo.soto@gmail.com>

  • attachment iterables3.patch added.

Uses is_iterable on django.template too. Also changes the order of the typical check for the common case: now we first check for non-strings, next for iterables.

09/14/07 14:11:41 changed by leosoto <leo.soto@gmail.com>

  • attachment iterables3.2.patch added.

The previous patch file is messed up. Now it's OK.

09/14/07 14:16:50 changed by Fredrik Lundh <fredrik@pythonware.com>

  • keywords changed from jython to jython, sprintsept14.
  • stage changed from Accepted to Ready for checkin.

Looks good to me. If you've missed something else, we can deal with that in an additional issue. Boldly tagging this and marking it as "Read for checkin".

09/14/07 14:55:25 changed by jacob

  • status changed from new to closed.
  • resolution set to fixed.

(In [6211]) Fixed #5445: added some compatibility code for the lack of iter in Jython 2.2. Thanks, Leo Soto.

Add/Change #5445 (Checking iterables by looking for a __iter__ attribute doesn't work on Jython)




Change Properties
Action