Re: [Xen-devel] [PATCH for-next RFC 3/4] pygrub: convert python files with 2to3

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/03/2019 11:31, Wei Liu wrote:
> On Tue, Mar 05, 2019 at 05:51:04PM +0000, Andrew Cooper wrote:
>> On 05/03/2019 16:42, Wei Liu wrote:
>>> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>>> ---
>>> Not sure this works with python 2.4, but it should work with 2.7 since
>>> the changes look more or less in the same vein as the changes in
>>> libxl.
>>>
>>> The conversion of the import is interesting. This definitely needs
>>> some testing.
>>> ---
>>>  tools/pygrub/src/ExtLinuxConf.py | 16 ++++++++--------
>>>  tools/pygrub/src/GrubConf.py     | 36 ++++++++++++++++++------------------
>>>  tools/pygrub/src/LiloConf.py     | 16 ++++++++--------
>>>  3 files changed, 34 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/tools/pygrub/src/ExtLinuxConf.py 
>>> b/tools/pygrub/src/ExtLinuxConf.py
>>> index d1789bf020..60da960c4b 100644
>>> --- a/tools/pygrub/src/ExtLinuxConf.py
>>> +++ b/tools/pygrub/src/ExtLinuxConf.py
>>> @@ -12,7 +12,7 @@
>>>  
>>>  import sys, re, os
>>>  import logging
>>> -import GrubConf
>>> +from . import GrubConf
>> Relative imports definitely don't exist in Py 2.4
>>
>>>  
>>>  class ExtLinuxImage(object):
>>>      def __init__(self, lines, path):
>>> @@ -32,7 +32,7 @@ class ExtLinuxImage(object):
>>>          self.lines = []
>>>          self.path = path
>>>          self.root = ""
>>> -        map(self.set_from_line, lines)
>>> +        list(map(self.set_from_line, lines))
>> This an abuse of map() in the first place, but the automatic
>> transformation makes the result even more confusing.
> Right. I tried to find the justification for this transformation but the
> document doesn't provide that.

The expected use of map is in the form:

x = map(fn, y)

which would leave x as a list in Py2, and a generator in Py3.

In most code, wrapping map with list() is the correct transformation to
make, because a) a lot of code written for Py2 expects it to be a list
and b) you cant programmatically evaluate whether leaving it in its
generator form is safe in context.

For this piece of code (and the other similar examples), map() is not
the correct construct to use in the first place, and probably wants
fixing for clarity alone, irrespective of the Py3 transformation.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel