Opened 4 years ago

Closed 4 years ago

Last modified 22 months ago

#25225 closed Cleanup/optimization (fixed)

Unnecessary/Redundant __iter__ method with ListMixin class (contrib.gis.geos.mutable_list.py)?

Reported by: Joshua Bixby Owned by: nobody
Component: GIS Version: 1.8
Severity: Normal Keywords: __iter__ geos gis ListMixin
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

The ListMixin class (contrib.gis.geos.mutable_list.py) currently has an __iter__ method and __getitem__ method. Typically iteration is implemented using one or the other, not both. Looking at the two methods, the __iter__method appears to be unnecessary since the Python interpreter will use __getitem__to iterate if __iter__ is not present. Also, stepping through code appears to have __iter__ call __getitem__, which raises the question of why define an __iter__ method when the interpreter can use __getitem__directly.

Currently, __getitem__ can't be used properly for iteration because of the custom IndexError raised in the _checkindex method. Switching from raising a django.contrib.gis.geos.error.GEOSIndexError exception to a exceptions.IndexError will allow for the interpreter to use __getitem__for iteration, which would allow removing the unnecessary or redundant __iter__ method.

Change History (6)

comment:1 Changed 4 years ago by Tim Graham

Description: modified (diff)

comment:2 Changed 4 years ago by Tim Graham

Easy pickings: unset

Could you propose a patch with a regression test?

comment:3 Changed 4 years ago by Tim Graham

It looks like #4740 is the reason the way this code is the way it is, but the template engine lookups now catch IndexError too, so we may be able to remove the custom exception.

comment:4 Changed 4 years ago by Tim Graham

Has patch: set
Triage Stage: UnreviewedAccepted

comment:5 Changed 4 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 197b187:

Fixed #25225 -- Simplified code to remove GEOSIndexError

The test is a regression for refs #4740 to show that the original
fix of GEOSIndexError is no longer needed.

comment:6 Changed 22 months ago by Tim Graham <timograham@…>

In 35800ac:

Refs #25225 -- Removed test for removed ListMixin._IndexError.

Unused since 197b1878105504b5ac7e399e1f52a6093c88baa3.

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