qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Cleber Rosa <crosa@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 5/9] iotests: Different iterator behavior in Python 3
Date: Fri, 19 Oct 2018 11:42:07 +0200	[thread overview]
Message-ID: <0a3afbfc-ee1f-402a-1f92-2355482876cb@redhat.com> (raw)
In-Reply-To: <84df80ea-f9c8-f953-9e4d-f4af8351edf2@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2150 bytes --]

On 16.10.18 00:39, Cleber Rosa wrote:
> 
> 
> On 10/15/18 10:14 AM, Max Reitz wrote:
>> In Python 3, several functions now return iterators instead of lists.
>> This includes range(), items(), map(), and filter().  This means that if
>> we really want a list, we have to wrap those instances with list().  On
>> the other hand, sometimes we do just want an iterator, in which case we
>> have sometimes used xrange() and iteritems() which no longer exist in
>> Python 3.  Just change these calls to be range() and items(), which
>> costs a bit of performance in Python 2, but will do the right thing in
>> Python 3 (which is what is important).
>>
>> In one instance, we only wanted the first instance of the result of a
>> filter() call.  Instead of using next(filter()) which would work only in
>> Python 3, or list(filter())[0] which would work everywhere but is a bit
>> weird, this instance is changed to a single-line for with next() wrapped
>> around, which works both in 2.7 and 3.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/044 | 12 ++++++------
>>  tests/qemu-iotests/056 |  2 +-
>>  tests/qemu-iotests/065 |  4 ++--
>>  tests/qemu-iotests/124 |  4 ++--
>>  tests/qemu-iotests/139 |  2 +-
>>  tests/qemu-iotests/163 |  6 +++---
>>  6 files changed, 15 insertions(+), 15 deletions(-)
>>
> 
> You have 2 files here which use xrange (which is a manageable size, and
> whose occurrences involve a moderate size of items) to also consider:
> 
> if sys.version_info.major == 2:
>    range = xrange

I don't think it's necessary, but it's so easy to grep for
"sys.version_info" once we want to completely switch to Python 3 that I
can't find good arguments against it. O:-)

Max

> Defaulting to the Python 3 names, but behaving the same across Python 2
> and 3.
> 
> To do the same for dict.iteritems() => dict.items() requires a lot more
> code, so I'd stay away from it for now.  Also, it looks like most of
> those dicts are small in size (**kwargs and the like).
> 
> Other than that suggestion,
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2018-10-19  9:42 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-15 14:14 [Qemu-devel] [PATCH 0/9] iotests: Make them work for both Python 2 and 3 Max Reitz
2018-10-15 14:14 ` [Qemu-devel] [PATCH 1/9] iotests: Make nbd-fault-injector flush Max Reitz
2018-10-15 19:42   ` Eduardo Habkost
2018-10-15 20:24   ` Cleber Rosa
2018-10-16 18:07   ` Eric Blake
2018-10-19  9:48     ` Max Reitz
2018-10-19 14:21       ` Eric Blake
2018-10-15 14:14 ` [Qemu-devel] [PATCH 2/9] iotests: Flush in iotests.py's QemuIoInteractive Max Reitz
2018-10-15 19:43   ` Eduardo Habkost
2018-10-15 20:49   ` Cleber Rosa
2018-10-15 14:14 ` [Qemu-devel] [PATCH 3/9] iotests: Use Python byte strings where appropriate Max Reitz
2018-10-15 19:53   ` Eduardo Habkost
2018-10-19  8:46     ` Max Reitz
2018-10-15 22:08   ` Philippe Mathieu-Daudé
2018-10-15 14:14 ` [Qemu-devel] [PATCH 4/9] iotests: Use // for Python integer division Max Reitz
2018-10-15 19:54   ` Eduardo Habkost
2018-10-15 21:13   ` Cleber Rosa
2018-10-19  9:06     ` Max Reitz
2018-10-15 14:14 ` [Qemu-devel] [PATCH 5/9] iotests: Different iterator behavior in Python 3 Max Reitz
2018-10-15 20:07   ` Eduardo Habkost
2018-10-19  8:52     ` Max Reitz
2018-10-15 22:39   ` Cleber Rosa
2018-10-19  9:42     ` Max Reitz [this message]
2018-10-15 14:14 ` [Qemu-devel] [PATCH 6/9] iotests: Explicitly inherit FDs in Python Max Reitz
2018-10-15 20:30   ` Eduardo Habkost
2018-10-19  9:03     ` Max Reitz
2018-10-15 23:18   ` Cleber Rosa
2018-10-19  9:43     ` Max Reitz
2018-10-15 14:14 ` [Qemu-devel] [PATCH 7/9] iotests: 'new' module replacement in 169 Max Reitz
2018-10-15 21:13   ` Eduardo Habkost
2018-10-15 23:38   ` Cleber Rosa
2018-10-15 23:57     ` Eduardo Habkost
2018-10-16  1:01       ` Cleber Rosa
2018-10-19  9:46         ` Max Reitz
2018-10-19 14:18           ` Eduardo Habkost
2018-10-15 14:14 ` [Qemu-devel] [PATCH 8/9] iotests: Modify imports for Python 3 Max Reitz
2018-10-15 18:59   ` Cleber Rosa
2018-10-15 20:15     ` Eduardo Habkost
2018-10-19  8:44     ` Max Reitz
2018-10-15 21:17   ` Eduardo Habkost
2018-10-16  0:05     ` Cleber Rosa
2018-10-16  0:12       ` Eduardo Habkost
2018-10-19  9:25         ` Max Reitz
2018-10-15 14:14 ` [Qemu-devel] [PATCH 9/9] iotests: Unify log outputs between Python 2 and 3 Max Reitz
2018-10-15 22:26   ` Eduardo Habkost
2018-10-19  9:33     ` Max Reitz
2018-10-15 22:19 ` [Qemu-devel] [PATCH 0/9] iotests: Make them work for both " Philippe Mathieu-Daudé
2018-10-19  9:08   ` Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a3afbfc-ee1f-402a-1f92-2355482876cb@redhat.com \
    --to=mreitz@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).