SF.net SVN: matplotlib:[6217] trunk/matplotlib/lib/matplotlib/axes.py

Manuel,

Although it doesn't hurt, I don't think it is worthwhile changing range to xrange. From the 2.5 docs:

xrange( [start,] stop[, step])
  This function is very similar to range(), but returns an ``xrange object'' instead of a list. This is an opaque sequence type which yields the same values as the corresponding list, without actually storing them all simultaneously. The advantage of xrange() over range() is minimal (since xrange() still has to create the values when asked for them) except when a very large range is used on a memory-starved machine or when all of the range's elements are never used (such as when the loop is usually terminated with break).

Note "minimal" advantage. xrange was intended for special-case use, not general use.

And from Python 3.0, http://docs.python.org/dev/3.0/whatsnew/3.0.html

xrange() renamed to range(), so range() will no longer produce a list but an iterable yielding integers when iterated over.

This implies to me that range is the preferred form, and xrange is intended to go away.

Eric

mmetz_bn@...189... wrote:

···

Revision: 6217
          http://matplotlib.svn.sourceforge.net/matplotlib/?rev=6217&view=rev
Author: mmetz_bn
Date: 2008-10-16 11:15:25 +0000 (Thu, 16 Oct 2008)

Log Message:
-----------
minor optimizations: replacing range by xrange in for loops

Modified Paths:
--------------
    trunk/matplotlib/lib/matplotlib/axes.py

Modified: trunk/matplotlib/lib/matplotlib/axes.py

--- trunk/matplotlib/lib/matplotlib/axes.py 2008-10-15 20:53:09 UTC (rev 6216)
+++ trunk/matplotlib/lib/matplotlib/axes.py 2008-10-16 11:15:25 UTC (rev 6217)
@@ -243,7 +243,7 @@
         x, y, multicol = self._xy_from_y(y)
          if multicol:
- for j in range(y.shape[1]):
+ for j in xrange(y.shape[1]):
                 color = self._get_next_cycle_color()
                 seg = mlines.Line2D(x, y[:,j],
                              color = color,
@@ -285,7 +285,7 @@
                 ret.append(seg)
              if multicol:
- for j in range(y.shape[1]):
+ for j in xrange(y.shape[1]):
                     makeline(x[:,j], y[:,j])
             else:
                 makeline(x, y)
@@ -324,7 +324,7 @@
                 closed = kwargs.get('closed', True)
                 func = makefill
             if multicol:
- for j in range(y.shape[1]):
+ for j in xrange(y.shape[1]):
                     func(x[:,j], y[:,j])
             else:
                 func(x, y)
@@ -372,7 +372,7 @@
             func = makefill
          if multicol:
- for j in range(y.shape[1]):
+ for j in xrange(y.shape[1]):
                 func(x[:,j], y[:,j])
         else:
             func(x, y)
@@ -3897,9 +3897,9 @@
             pass
         elif align == 'center':
             if orientation == 'vertical':
- left = [left[i] - width[i]/2. for i in range(len(left))]
+ left = [left[i] - width[i]/2. for i in xrange(len(left))]
             elif orientation == 'horizontal':
- bottom = [bottom[i] - height[i]/2. for i in range(len(bottom))]
+ bottom = [bottom[i] - height[i]/2. for i in xrange(len(bottom))]
          else:
             raise ValueError, 'invalid alignment: %s' % align
@@ -4571,7 +4571,7 @@
                 elif nc == 1:
                     x = [x.ravel()]
                 else:
- x = [x[:,i] for i in range(nc)]
+ x = [x[:,i] for i in xrange(nc)]
             else:
                 raise ValueError, "input x can have no more than 2 dimensions"
         if not hasattr(x[0], '__len__'):
@@ -4665,7 +4665,7 @@
             else:
                 def doplot(*args):
                     shuffled = []
- for i in range(0, len(args), 3):
+ for i in xrange(0, len(args), 3):
                         shuffled.extend([args[i+1], args[i], args[i+2]])
                     return self.plot(*shuffled)

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Matplotlib-checkins mailing list
Matplotlib-checkins@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-checkins

Please see the end of the mail for the important point !!!

Eric Firing wrote:

Manuel,

Although it doesn't hurt, I don't think it is worthwhile changing range
to xrange. From the 2.5 docs:

[...snip...]

Note "minimal" advantage. xrange was intended for special-case use, not
general use.

Eric,

yes, I absolutely agree with you that this is only a small (minimal)
advantage, probably not worth to worry about. Nevertheless ...

And from Python 3.0, http://docs.python.org/dev/3.0/whatsnew/3.0.html
xrange() renamed to range(), so range() will no longer produce a list
but an iterable yielding integers when iterated over.

Python 3.0 will use xrange() by default, but it is then named range(),
so from that _I_ conclude that xrange() should be used by default. You
can also see the difference by using 2to3:

"""
for i in range(10): print i
for i in xrange(10): print i
"""

gets converted to:

"""
for i in range(10): print i
for i in range(10): print i
"""

That is, because 2to3 is a clever program. But:

"""
a = range(10)
b = xrange(10)
for i in a: print i
for i in b: print i
"""

gets converted to

"""
a = list(range(10))
b = range(10)
for i in a: print(i)
for i in b: print(i)
"""

:wink:

As you said, it's only a minimal advantage and 2to3 is a clever code!!!

(THE IMPORTANT POINT)

But this brings me to another, more important point: In the axes hist()
method, a keyword named "range" is used that is passed to the numpy
histogram() function, which has the kwarg 'range'. Now, this is not a
problem as long as the range() builtin function is not used in the
hist() method. But there are a few loops in this method that use
xrange(), so this code will be translated to range() in py3 -- and that
will be a problem. A basic example with a pseudo-code:

"""
def foo(x, range=(1,10)):
    print range
    for i in xrange(x): print i
foo(10)
"""

with 2to3 -->

"""
def foo(x, range=(1,10)):
    print(range)
    for i in range(x): print(i)
foo(10)
"""
which then fails.

One solution would be to use a different keyword argument, maybe
"binrange" instead of "range" and to throw a deprecated warning for the
old keyword ???

Manuel

This implies to me that range is the preferred form, and xrange is
intended to go away.

Eric

[...snip...]

Manuel Metz wrote:

Please see the end of the mail for the important point !!!

Thank you--I see you are way ahead of me on this. See comments below.

Eric Firing wrote:

Manuel,

Although it doesn't hurt, I don't think it is worthwhile changing range to xrange. From the 2.5 docs:

[...snip...]

Note "minimal" advantage. xrange was intended for special-case use, not general use.

Eric,

yes, I absolutely agree with you that this is only a small (minimal)
advantage, probably not worth to worry about. Nevertheless ...

And from Python 3.0, http://docs.python.org/dev/3.0/whatsnew/3.0.html
xrange() renamed to range(), so range() will no longer produce a list but an iterable yielding integers when iterated over.

Python 3.0 will use xrange() by default, but it is then named range(),
so from that _I_ conclude that xrange() should be used by default. You
can also see the difference by using 2to3:

"""
for i in range(10): print i
for i in xrange(10): print i
"""

gets converted to:

"""
for i in range(10): print i
"""

That is, because 2to3 is a clever program. But:

"""
a = range(10)
b = xrange(10)
for i in a: print i
for i in b: print i
"""

gets converted to

"""
a = list(range(10))
b = range(10)
for i in a: print(i)
for i in b: print(i)
"""

:wink:

As you said, it's only a minimal advantage and 2to3 is a clever code!!!

I am glad you brought the above to my attention--it completely changes my point of view. It does appear that changing to xrange now, whenever it will work (that is, when one does not *need* a list) will make the transition to Python 3 more efficient and has no disadvantage with present code.

(THE IMPORTANT POINT)

But this brings me to another, more important point: In the axes hist()
method, a keyword named "range" is used that is passed to the numpy
histogram() function, which has the kwarg 'range'. Now, this is not a
problem as long as the range() builtin function is not used in the
hist() method. But there are a few loops in this method that use
xrange(), so this code will be translated to range() in py3 -- and that
will be a problem. A basic example with a pseudo-code:

"""
def foo(x, range=(1,10)):
    print range
    for i in xrange(x): print i
foo(10)
"""

with 2to3 -->

"""
def foo(x, range=(1,10)):
    print(range)
    for i in range(x): print(i)
foo(10)
"""
which then fails.

One solution would be to use a different keyword argument, maybe
"binrange" instead of "range" and to throw a deprecated warning for the
old keyword ???

Yes, I think the use of any builtin as a kwarg is a bug that should be squashed via a new kwarg with a deprecation. Similarly, use of any builtin as in internal variable should be considered a latent bug and fixed.

Unfortunately, in this case, the badly-named kwarg is in numpy.histogram as well. The best thing would be to try to get the same change made in numpy so that mpl hist and numpy.histogram kwargs would stay in sync.

To make matters worse, histogram has evolved in such a way that its kwargs are a confusing mess. It is too bad that when the "new" syntax was developed, the "range" kwarg was not changed at the same time. I don't know whether any more changes would be accepted now.

If there is to be a new kwarg, I think I would call it "cliprange", since it is essentially used to clip the input--unless "new" is not True. It is not really a "bin range", because it can be set independently of the bins. (I have not traced the code; I am basing my statement on the docstring, so I could be wrong about what the code actually does.)

Eric

···

Manuel

This implies to me that range is the preferred form, and xrange is intended to go away.

Eric

[...snip...]

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Manuel Metz wrote:

Please see the end of the mail for the important point !!!

Thank you--I see you are way ahead of me on this. See comments below.

Eric Firing wrote:

Manuel,

Although it doesn't hurt, I don't think it is worthwhile changing range
to xrange. From the 2.5 docs:

[...snip...]

Note "minimal" advantage. xrange was intended for special-case use, not
general use.

Eric,

yes, I absolutely agree with you that this is only a small (minimal)
advantage, probably not worth to worry about. Nevertheless ...

And from Python 3.0, http://docs.python.org/dev/3.0/whatsnew/3.0.html
xrange() renamed to range(), so range() will no longer produce a list
but an iterable yielding integers when iterated over.

Python 3.0 will use xrange() by default, but it is then named range(),
so from that _I_ conclude that xrange() should be used by default. You
can also see the difference by using 2to3:

"""
for i in range(10): print i
for i in xrange(10): print i
"""

gets converted to:

"""
for i in range(10): print i
for i in range(10): print i
"""

That is, because 2to3 is a clever program. But:

"""
a = range(10)
b = xrange(10)
for i in a: print i
for i in b: print i
"""

gets converted to

"""
a = list(range(10))
b = range(10)
for i in a: print(i)
for i in b: print(i)
"""

:wink:

As you said, it's only a minimal advantage and 2to3 is a clever code!!!

I am glad you brought the above to my attention--it completely changes
my point of view. It does appear that changing to xrange now, whenever
it will work (that is, when one does not *need* a list) will make the
transition to Python 3 more efficient and has no disadvantage with
present code.

(THE IMPORTANT POINT)

But this brings me to another, more important point: In the axes hist()
method, a keyword named "range" is used that is passed to the numpy
histogram() function, which has the kwarg 'range'. Now, this is not a
problem as long as the range() builtin function is not used in the
hist() method. But there are a few loops in this method that use
xrange(), so this code will be translated to range() in py3 -- and that
will be a problem. A basic example with a pseudo-code:

The histogram function contains the code for the old behaviour and the
new behaviour side
by side. Now only the old behaviour uses xrange, while the new
behaviour uses numpy's arange.
The old behaviour will be gone altogether in release 1.4.

"""
def foo(x, range=(1,10)):
    print range
    for i in xrange(x): print i
foo(10)
"""

with 2to3 -->

"""
def foo(x, range=(1,10)):
    print(range)
    for i in range(x): print(i)
foo(10)
"""
which then fails.

One solution would be to use a different keyword argument, maybe
"binrange" instead of "range" and to throw a deprecated warning for the
old keyword ???

Yes, I think the use of any builtin as a kwarg is a bug that should be
squashed via a new kwarg with a deprecation. Similarly, use of any
builtin as in internal variable should be considered a latent bug and fixed.

Unfortunately, in this case, the badly-named kwarg is in numpy.histogram
as well. The best thing would be to try to get the same change made in
numpy so that mpl hist and numpy.histogram kwargs would stay in sync.

To make matters worse, histogram has evolved in such a way that its
kwargs are a confusing mess. It is too bad that when the "new" syntax
was developed, the "range" kwarg was not changed at the same time.

Indeed.

I don't know whether any more changes would be accepted now.

I would oppose any change to histogram calling convention that does not
fix a critical bug. I agree that using a built-in name as an argument is
a bug, but I believe it is the lesser evil compared to asking users to
change their code
again.

I've added a note in numpy ticket 797 suggesting that a py3k
compatible numpy version
should have the old behaviour removed to avoid the name clash.

Thanks for bringing this up,

Regards,

David

···

On Sat, Oct 18, 2008 at 3:07 PM, Eric Firing <efiring@...229...> wrote:

If there is to be a new kwarg, I think I would call it "cliprange",
since it is essentially used to clip the input--unless "new" is not
True. It is not really a "bin range", because it can be set
independently of the bins. (I have not traced the code; I am basing my
statement on the docstring, so I could be wrong about what the code
actually does.)

Eric

Manuel

This implies to me that range is the preferred form, and xrange is
intended to go away.

Eric

[...snip...]

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

I think in this case we have to change it, but we can do it gently.
Ie, for a release cycle, if we detect "range" in the kwarg case, we'll
set the correct kwarg, eg "binrange", issue a warning, but not raise
an error. That way users can fairly easily change their code w/o
breakage. Whoever does the deprecation should note the date and
version of the deprecation warning, so we will know when enough time
and releases have passed to remove range entirely.

JDH

···

On Mon, Oct 20, 2008 at 9:01 AM, David Huard <david.huard@...149...> wrote:

I would oppose any change to histogram calling convention that does not
fix a critical bug. I agree that using a built-in name as an argument is
a bug, but I believe it is the lesser evil compared to asking users to
change their code
again.

I would oppose any change to histogram calling convention that does not
fix a critical bug. I agree that using a built-in name as an argument is
a bug, but I believe it is the lesser evil compared to asking users to
change their code
again.

It may not have been clear enough, but let me clarify that I was
opposing a change to numpy.histogram, not meddling in matplotlib's
hist.

Cheers,
David

···

On Mon, Oct 20, 2008 at 10:19 AM, John Hunter <jdh2358@...149...> wrote:

On Mon, Oct 20, 2008 at 9:01 AM, David Huard <david.huard@...149...> wrote:

I think in this case we have to change it, but we can do it gently.
Ie, for a release cycle, if we detect "range" in the kwarg case, we'll
set the correct kwarg, eg "binrange", issue a warning, but not raise
an error. That way users can fairly easily change their code w/o
breakage. Whoever does the deprecation should note the date and
version of the deprecation warning, so we will know when enough time
and releases have passed to remove range entirely.

JDH

David Huard wrote:

I would oppose any change to histogram calling convention that does not
fix a critical bug. I agree that using a built-in name as an argument is
a bug, but I believe it is the lesser evil compared to asking users to
change their code
again.

It may not have been clear enough, but let me clarify that I was
opposing a change to numpy.histogram, not meddling in matplotlib's
hist.

David,

   the only thing is that *if* we change the keyword in matplotlib, we should change it to the same keyword that will (probably) be in numpy.

Put it the other way: I think it would be good to agree on a new keyword ("binrange", "cliprange", ...) and then change it gently in both projects, numpy and matplotlib, as John suggested (or alternatively leave the keyword as is, and use the arange workaround, but I personally don't like this idea).

Cheers,
   Manuel

···

On Mon, Oct 20, 2008 at 10:19 AM, John Hunter <jdh2358@...149...> wrote:

On Mon, Oct 20, 2008 at 9:01 AM, David Huard <david.huard@...149...> wrote:

Cheers,
David

I think in this case we have to change it, but we can do it gently.
Ie, for a release cycle, if we detect "range" in the kwarg case, we'll
set the correct kwarg, eg "binrange", issue a warning, but not raise
an error. That way users can fairly easily change their code w/o
breakage. Whoever does the deprecation should note the date and
version of the deprecation warning, so we will know when enough time
and releases have passed to remove range entirely.

JDH

David Huard wrote:

I would oppose any change to histogram calling convention that does not
fix a critical bug. I agree that using a built-in name as an argument is
a bug, but I believe it is the lesser evil compared to asking users to
change their code
again.

It may not have been clear enough, but let me clarify that I was
opposing a change to numpy.histogram, not meddling in matplotlib's
hist.

David,

  the only thing is that *if* we change the keyword in matplotlib, we
should change it to the same keyword that will (probably) be in numpy.

Put it the other way: I think it would be good to agree on a new keyword
("binrange", "cliprange", ...) and then change it gently in both
projects, numpy and matplotlib, as John suggested (or alternatively
leave the keyword as is, and use the arange workaround, but I personally
don't like this idea).

Cheers,
  Manuel

Hi Manuel, John and Eric,

What was the time line you had in mind for the change ?

My main concern is to avoid antagonizing the folks who were annoyed by
the semantic change to histogram. Some of them might not be happy to
go back and make another change to histogram in their code so soon
after having dealt with the `new` keyword, even if you are gentle
about it.

My second concern is to avoid breaking the interface in a minor
release. This would mean waiting for NumPy 2.0 before completely
removing the range argument.

David

···

On Tue, Oct 21, 2008 at 3:31 AM, Manuel Metz <mmetz@...459...> wrote:

On Mon, Oct 20, 2008 at 10:19 AM, John Hunter <jdh2358@...149...> wrote:

On Mon, Oct 20, 2008 at 9:01 AM, David Huard <david.huard@...149...> wrote:

Cheers,
David

I think in this case we have to change it, but we can do it gently.
Ie, for a release cycle, if we detect "range" in the kwarg case, we'll
set the correct kwarg, eg "binrange", issue a warning, but not raise
an error. That way users can fairly easily change their code w/o
breakage. Whoever does the deprecation should note the date and
version of the deprecation warning, so we will know when enough time
and releases have passed to remove range entirely.

JDH

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel