Opened 10 months ago

Closed 9 months ago

#3757 closed defect (fixed)

Python3: range vs xrange

Reported by: neteler Owned by: grass-dev@…
Priority: normal Milestone: 7.8.0
Component: Python Version: svn-trunk
Keywords: python3 Cc:
CPU: Unspecified Platform: Unspecified

Description

In r74088 xrange() was changed to range(). If that change is fine, should these other xrange() usages also be changed?

ag --python " in xrange" 
doc/python/raster_example_ctypes.py
67:for row_n in xrange(rows):

lib/python/temporal/abstract_space_time_dataset.py
1146:                >>> for i in xrange(3):

lib/python/temporal/spatio_temporal_relationships.py
423:#        for i in xrange(len(maps)):
425:#            for j in xrange(offset, len(maps)):

lib/python/pygrass/modules/interface/module.py
64:    >>> for i in xrange(5):
87:    >>> for i in xrange(5):
111:    >>> for i in xrange(3):

lib/python/pygrass/raster/__init__.py
412:            >>> for row in xrange(map_a.info.rows):
442:            >>> for i in xrange(4):
479:            >>> for row in xrange(map_a.info.rows):
480:            ...     for col in xrange(map_a.info.cols):

lib/python/pygrass/vector/basic.py
384:    >>> for cat in xrange(100, 110): cats.set(cat, layer=cat-50)

lib/python/pygrass/vector/__init__.py
87:        #return (self.read(f_id) for f_id in xrange(self.num_of_features()))

Change History (4)

comment:1 Changed 10 months ago by pmav99

Python 2 has both range() and xrange(). In Python 3, range() has been removed and xrange() has been renamed to range().

So any code using xrange() is not Python 3 compatible, so the answer is yes xrange() needs to be replaced by range(). This does lead to bigger RAM usage (range() creates a list while xrange() creates an iterator, although in the vast majority of the cases the impact is negligible (and when it isn't you should probably rethink your algorithms).

The alternative is to introduce a "compatibility layer". E.g.

import sys

if sys.version_info.major < 3:
    range = xrange

but this needs to be used in every module (or added to a compat.py module and imported from any other module. Moreover, this can break something (if people were abusing range() that is).

All that being said, when cross-version compatibility is required it is usually a good idea to use either six and/or future. I believe six is already being pulled by GRASS dependencies (matptlotlib if memory serves).

Version 2, edited 10 months ago by pmav99 (previous) (next) (diff)

comment:2 Changed 10 months ago by annakrat

For the record, r74100 already replaces xrange with range in all important places. All these usages left are in documentation/comments so it's less important, I didn't do it mainly because I have some local changes so it was inconvenient.

comment:3 Changed 10 months ago by pmav99

It seems that there are some more instances that should be changed:

$ ag xrange --python  -l

scripts/i.spectral/i.spectral.py
gui/wxpython/timeline/frame.py
gui/wxpython/nviz/wxnviz.py
gui/wxpython/iclass/plots.py
gui/wxpython/gui_core/wxlibplot.py
gui/wxpython/tplot/frame.py
temporal/t.rast.accdetect/t.rast.accdetect.py
tools/ppmrotate.py
doc/python/raster_example_ctypes.py
lib/python/temporal/spatio_temporal_relationships.py
lib/python/temporal/abstract_space_time_dataset.py
lib/python/pygrass/raster/__init__.py
lib/python/pygrass/vector/__init__.py
lib/python/pygrass/vector/basic.py
lib/python/pygrass/modules/interface/module.py
Last edited 10 months ago by pmav99 (previous) (diff)

comment:4 Changed 9 months ago by annakrat

Resolution: fixed
Status: newclosed

In 74128:

fix #3757: replace xrange with range

Note: See TracTickets for help on using tickets.