Commit d30a1ad0443 translated the symbol file checker from perl to
python by doing a literal translation in most cases. Unfortunately one
string formatting operation was not really translated into python
leaving users with non-helpful error:
'Symbol $1 is listed twice'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
scripts/check-symfile.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
index 0c02591991..34396b8623 100755
--- a/scripts/check-symfile.py
+++ b/scripts/check-symfile.py
@@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
line = line.strip(";")
if line in wantsyms:
- print("Symbol $1 is listed twice", file=sys.stderr)
+ print("Symbol %s is listed twice" % line ,file=sys.stderr)
ret = 1
else:
wantsyms[line] = True
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/25/19 4:37 PM, Peter Krempa wrote: > Commit d30a1ad0443 translated the symbol file checker from perl to > python by doing a literal translation in most cases. Unfortunately one > string formatting operation was not really translated into python > leaving users with non-helpful error: > > 'Symbol $1 is listed twice' > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- > scripts/check-symfile.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
> Commit d30a1ad0443 translated the symbol file checker from perl to
> python by doing a literal translation in most cases. Unfortunately one
> string formatting operation was not really translated into python
> leaving users with non-helpful error:
>
> 'Symbol $1 is listed twice'
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> scripts/check-symfile.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
> index 0c02591991..34396b8623 100755
> --- a/scripts/check-symfile.py
> +++ b/scripts/check-symfile.py
> @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
> line = line.strip(";")
>
> if line in wantsyms:
> - print("Symbol $1 is listed twice", file=sys.stderr)
> + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Not a deal breaker, but IMO should at least the "new" syntax for string
formatting using the .format() method (works both with python 2 and 3).
Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
started using string interpolation (or f-strings if you want).
Reviewed-by: Erik Skultety <eskultet@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Nov 25, 2019 at 16:58:39 +0100, Erik Skultety wrote:
> On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
> > Commit d30a1ad0443 translated the symbol file checker from perl to
> > python by doing a literal translation in most cases. Unfortunately one
> > string formatting operation was not really translated into python
> > leaving users with non-helpful error:
> >
> > 'Symbol $1 is listed twice'
> >
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > scripts/check-symfile.py | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
> > index 0c02591991..34396b8623 100755
> > --- a/scripts/check-symfile.py
> > +++ b/scripts/check-symfile.py
> > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
> > line = line.strip(";")
> >
> > if line in wantsyms:
> > - print("Symbol $1 is listed twice", file=sys.stderr)
> > + print("Symbol %s is listed twice" % line ,file=sys.stderr)
>
> Not a deal breaker, but IMO should at least the "new" syntax for string
> formatting using the .format() method (works both with python 2 and 3).
This rest of this script uses the % syntax so I'm not going to add a
different style into this file nor do a conversion of irrelevant parts
in this patch.
Obviously if you have suggestions you can update the coding style
guidelines and/or convert the scripts to use the more pythonic syntax.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/25/19 4:58 PM, Erik Skultety wrote:
> On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
>> Commit d30a1ad0443 translated the symbol file checker from perl to
>> python by doing a literal translation in most cases. Unfortunately one
>> string formatting operation was not really translated into python
>> leaving users with non-helpful error:
>>
>> 'Symbol $1 is listed twice'
>>
>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> ---
>> scripts/check-symfile.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
>> index 0c02591991..34396b8623 100755
>> --- a/scripts/check-symfile.py
>> +++ b/scripts/check-symfile.py
>> @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
>> line = line.strip(";")
>>
>> if line in wantsyms:
>> - print("Symbol $1 is listed twice", file=sys.stderr)
>> + print("Symbol %s is listed twice" % line ,file=sys.stderr)
>
> Not a deal breaker, but IMO should at least the "new" syntax for string
> formatting using the .format() method (works both with python 2 and 3).
>
> Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
> started using string interpolation (or f-strings if you want).
Well, looks like we are not using that anywhere. And frankly, f-strings
are horrible. This is the most readable style for us, C developers IMO.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
> On 11/25/19 4:58 PM, Erik Skultety wrote:
> > On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
> > > Commit d30a1ad0443 translated the symbol file checker from perl to
> > > python by doing a literal translation in most cases. Unfortunately one
> > > string formatting operation was not really translated into python
> > > leaving users with non-helpful error:
> > >
> > > 'Symbol $1 is listed twice'
> > >
> > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > ---
> > > scripts/check-symfile.py | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
> > > index 0c02591991..34396b8623 100755
> > > --- a/scripts/check-symfile.py
> > > +++ b/scripts/check-symfile.py
> > > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
> > > line = line.strip(";")
> > >
> > > if line in wantsyms:
> > > - print("Symbol $1 is listed twice", file=sys.stderr)
> > > + print("Symbol %s is listed twice" % line ,file=sys.stderr)
> >
> > Not a deal breaker, but IMO should at least the "new" syntax for string
> > formatting using the .format() method (works both with python 2 and 3).
> >
> > Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
> > started using string interpolation (or f-strings if you want).
>
> Well, looks like we are not using that anywhere. And frankly, f-strings are
> horrible. This is the most readable style for us, C developers IMO.
Can you be more specific on what exactly is horrible about f-strings? IMO it's
actually very intuitive way of formatting strings unlike using the '%'
formatting sign where depending on whether you have 1 or multiple arguments you
may or may not need to use a tuple. F-strings are also a bit faster than the
other formatting methods and because they're evaluated during runtime, you can
evaluate arbitrary expressions, even call functions.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/26/19 9:24 AM, Erik Skultety wrote:
> On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
>> On 11/25/19 4:58 PM, Erik Skultety wrote:
>>> On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
>>>> Commit d30a1ad0443 translated the symbol file checker from perl to
>>>> python by doing a literal translation in most cases. Unfortunately one
>>>> string formatting operation was not really translated into python
>>>> leaving users with non-helpful error:
>>>>
>>>> 'Symbol $1 is listed twice'
>>>>
>>>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>>>> ---
>>>> scripts/check-symfile.py | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
>>>> index 0c02591991..34396b8623 100755
>>>> --- a/scripts/check-symfile.py
>>>> +++ b/scripts/check-symfile.py
>>>> @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
>>>> line = line.strip(";")
>>>>
>>>> if line in wantsyms:
>>>> - print("Symbol $1 is listed twice", file=sys.stderr)
>>>> + print("Symbol %s is listed twice" % line ,file=sys.stderr)
>>>
>>> Not a deal breaker, but IMO should at least the "new" syntax for string
>>> formatting using the .format() method (works both with python 2 and 3).
>>>
>>> Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
>>> started using string interpolation (or f-strings if you want).
>>
>> Well, looks like we are not using that anywhere. And frankly, f-strings are
>> horrible. This is the most readable style for us, C developers IMO.
>
> Can you be more specific on what exactly is horrible about f-strings? IMO it's
> actually very intuitive way of formatting strings unlike using the '%'
> formatting sign where depending on whether you have 1 or multiple arguments you
> may or may not need to use a tuple. F-strings are also a bit faster than the
> other formatting methods and because they're evaluated during runtime, you can
> evaluate arbitrary expressions, even call functions.
That's exactly what I find horrible. Just consider the following example:
print(f'a={f(x,n):d}, b={g(x,n):d}')
IMO the following is more readable:
print("a=%d, b=%d" % (f(x,n), g(x,n)))
Once again, I'm talking about C developers (me specifically). I don't
doubt that an experienced python developer finds f-strings a step forward.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
> On 11/26/19 9:24 AM, Erik Skultety wrote:
> > On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
> > > On 11/25/19 4:58 PM, Erik Skultety wrote:
> > > > On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
> > > > > Commit d30a1ad0443 translated the symbol file checker from perl to
> > > > > python by doing a literal translation in most cases. Unfortunately one
> > > > > string formatting operation was not really translated into python
> > > > > leaving users with non-helpful error:
> > > > >
> > > > > 'Symbol $1 is listed twice'
> > > > >
> > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > > > ---
> > > > > scripts/check-symfile.py | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
> > > > > index 0c02591991..34396b8623 100755
> > > > > --- a/scripts/check-symfile.py
> > > > > +++ b/scripts/check-symfile.py
> > > > > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
> > > > > line = line.strip(";")
> > > > >
> > > > > if line in wantsyms:
> > > > > - print("Symbol $1 is listed twice", file=sys.stderr)
> > > > > + print("Symbol %s is listed twice" % line ,file=sys.stderr)
> > > >
> > > > Not a deal breaker, but IMO should at least the "new" syntax for string
> > > > formatting using the .format() method (works both with python 2 and 3).
> > > >
> > > > Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
> > > > started using string interpolation (or f-strings if you want).
> > >
> > > Well, looks like we are not using that anywhere. And frankly, f-strings are
> > > horrible. This is the most readable style for us, C developers IMO.
> >
> > Can you be more specific on what exactly is horrible about f-strings? IMO it's
> > actually very intuitive way of formatting strings unlike using the '%'
> > formatting sign where depending on whether you have 1 or multiple arguments you
> > may or may not need to use a tuple. F-strings are also a bit faster than the
> > other formatting methods and because they're evaluated during runtime, you can
> > evaluate arbitrary expressions, even call functions.
>
> That's exactly what I find horrible. Just consider the following example:
>
> print(f'a={f(x,n):d}, b={g(x,n):d}')
>
> IMO the following is more readable:
>
> print("a=%d, b=%d" % (f(x,n), g(x,n)))
>
> Once again, I'm talking about C developers (me specifically). I don't doubt
> that an experienced python developer finds f-strings a step forward.
I've plenty of python dev experiance and I still think f-strings are
a horrible regression in design. IMHO it is a good thing to have the
parameters visually separated from the string, as it makes it clearer
to read the string being formatted.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 26, 2019 at 10:01:11AM +0000, Daniel P. Berrangé wrote:
> On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
> > On 11/26/19 9:24 AM, Erik Skultety wrote:
> > > On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
> > > > On 11/25/19 4:58 PM, Erik Skultety wrote:
> > > > > On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
> > > > > > Commit d30a1ad0443 translated the symbol file checker from perl to
> > > > > > python by doing a literal translation in most cases. Unfortunately one
> > > > > > string formatting operation was not really translated into python
> > > > > > leaving users with non-helpful error:
> > > > > >
> > > > > > 'Symbol $1 is listed twice'
> > > > > >
> > > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > > > > ---
> > > > > > scripts/check-symfile.py | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
> > > > > > index 0c02591991..34396b8623 100755
> > > > > > --- a/scripts/check-symfile.py
> > > > > > +++ b/scripts/check-symfile.py
> > > > > > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
> > > > > > line = line.strip(";")
> > > > > >
> > > > > > if line in wantsyms:
> > > > > > - print("Symbol $1 is listed twice", file=sys.stderr)
> > > > > > + print("Symbol %s is listed twice" % line ,file=sys.stderr)
> > > > >
> > > > > Not a deal breaker, but IMO should at least the "new" syntax for string
> > > > > formatting using the .format() method (works both with python 2 and 3).
> > > > >
> > > > > Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
> > > > > started using string interpolation (or f-strings if you want).
> > > >
> > > > Well, looks like we are not using that anywhere. And frankly, f-strings are
> > > > horrible. This is the most readable style for us, C developers IMO.
> > >
> > > Can you be more specific on what exactly is horrible about f-strings? IMO it's
> > > actually very intuitive way of formatting strings unlike using the '%'
> > > formatting sign where depending on whether you have 1 or multiple arguments you
> > > may or may not need to use a tuple. F-strings are also a bit faster than the
> > > other formatting methods and because they're evaluated during runtime, you can
> > > evaluate arbitrary expressions, even call functions.
> >
> > That's exactly what I find horrible. Just consider the following example:
> >
> > print(f'a={f(x,n):d}, b={g(x,n):d}')
> >
> > IMO the following is more readable:
> >
> > print("a=%d, b=%d" % (f(x,n), g(x,n)))
> >
> > Once again, I'm talking about C developers (me specifically). I don't doubt
> > that an experienced python developer finds f-strings a step forward.
>
> I've plenty of python dev experiance and I still think f-strings are
> a horrible regression in design. IMHO it is a good thing to have the
> parameters visually separated from the string, as it makes it clearer
> to read the string being formatted.
I tend to agree here. f-strings are not that bad but it allows you to
write ugly code. It is probably good for interactive python but
otherwise I would prefer the new-style .format() with named arguments:
"Hello {name}".format(name="Pavel")
The major benefit that I see in this style is that it helps translators
to figure out what the substitution will be and the order of arguments
doesn't matter and you can build the dictionary passed to format()
outside of it's call.
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 26, 2019 at 11:25:28AM +0100, Pavel Hrdina wrote:
> On Tue, Nov 26, 2019 at 10:01:11AM +0000, Daniel P. Berrangé wrote:
> > On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
> > > On 11/26/19 9:24 AM, Erik Skultety wrote:
> > > > On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
> > > > > On 11/25/19 4:58 PM, Erik Skultety wrote:
> > > > > > On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
> > > > > > > Commit d30a1ad0443 translated the symbol file checker from perl to
> > > > > > > python by doing a literal translation in most cases. Unfortunately one
> > > > > > > string formatting operation was not really translated into python
> > > > > > > leaving users with non-helpful error:
> > > > > > >
> > > > > > > 'Symbol $1 is listed twice'
> > > > > > >
> > > > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > > > > > > ---
> > > > > > > scripts/check-symfile.py | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
> > > > > > > index 0c02591991..34396b8623 100755
> > > > > > > --- a/scripts/check-symfile.py
> > > > > > > +++ b/scripts/check-symfile.py
> > > > > > > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
> > > > > > > line = line.strip(";")
> > > > > > >
> > > > > > > if line in wantsyms:
> > > > > > > - print("Symbol $1 is listed twice", file=sys.stderr)
> > > > > > > + print("Symbol %s is listed twice" % line ,file=sys.stderr)
> > > > > >
> > > > > > Not a deal breaker, but IMO should at least the "new" syntax for string
> > > > > > formatting using the .format() method (works both with python 2 and 3).
> > > > > >
> > > > > > Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
> > > > > > started using string interpolation (or f-strings if you want).
> > > > >
> > > > > Well, looks like we are not using that anywhere. And frankly, f-strings are
> > > > > horrible. This is the most readable style for us, C developers IMO.
> > > >
> > > > Can you be more specific on what exactly is horrible about f-strings? IMO it's
> > > > actually very intuitive way of formatting strings unlike using the '%'
> > > > formatting sign where depending on whether you have 1 or multiple arguments you
> > > > may or may not need to use a tuple. F-strings are also a bit faster than the
> > > > other formatting methods and because they're evaluated during runtime, you can
> > > > evaluate arbitrary expressions, even call functions.
> > >
> > > That's exactly what I find horrible. Just consider the following example:
> > >
> > > print(f'a={f(x,n):d}, b={g(x,n):d}')
> > >
> > > IMO the following is more readable:
> > >
> > > print("a=%d, b=%d" % (f(x,n), g(x,n)))
> > >
> > > Once again, I'm talking about C developers (me specifically). I don't doubt
> > > that an experienced python developer finds f-strings a step forward.
> >
> > I've plenty of python dev experiance and I still think f-strings are
> > a horrible regression in design. IMHO it is a good thing to have the
> > parameters visually separated from the string, as it makes it clearer
> > to read the string being formatted.
>
> I tend to agree here. f-strings are not that bad but it allows you to
> write ugly code. It is probably good for interactive python but
> otherwise I would prefer the new-style .format() with named arguments:
>
> "Hello {name}".format(name="Pavel")
>
> The major benefit that I see in this style is that it helps translators
> to figure out what the substitution will be and the order of arguments
> doesn't matter and you can build the dictionary passed to format()
> outside of it's call.
Note those advantages are already possible with traditional syntax
"Hello %(name)s" % { name: "Pavel"}
but for our build scripts this is academic as we're never going to
do translation of anything in them.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 26, 2019 at 10:29:14AM +0000, Daniel P. Berrangé wrote:
>On Tue, Nov 26, 2019 at 11:25:28AM +0100, Pavel Hrdina wrote:
>> On Tue, Nov 26, 2019 at 10:01:11AM +0000, Daniel P. Berrangé wrote:
>> > On Tue, Nov 26, 2019 at 09:48:31AM +0100, Michal Privoznik wrote:
>> > > On 11/26/19 9:24 AM, Erik Skultety wrote:
>> > > > On Mon, Nov 25, 2019 at 05:17:54PM +0100, Michal Privoznik wrote:
>> > > > > On 11/25/19 4:58 PM, Erik Skultety wrote:
>> > > > > > On Mon, Nov 25, 2019 at 04:37:36PM +0100, Peter Krempa wrote:
>> > > > > > > Commit d30a1ad0443 translated the symbol file checker from perl to
>> > > > > > > python by doing a literal translation in most cases. Unfortunately one
>> > > > > > > string formatting operation was not really translated into python
>> > > > > > > leaving users with non-helpful error:
>> > > > > > >
>> > > > > > > 'Symbol $1 is listed twice'
>> > > > > > >
>> > > > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> > > > > > > ---
>> > > > > > > scripts/check-symfile.py | 2 +-
>> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > > > > > >
>> > > > > > > diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
>> > > > > > > index 0c02591991..34396b8623 100755
>> > > > > > > --- a/scripts/check-symfile.py
>> > > > > > > +++ b/scripts/check-symfile.py
>> > > > > > > @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
>> > > > > > > line = line.strip(";")
>> > > > > > >
>> > > > > > > if line in wantsyms:
>> > > > > > > - print("Symbol $1 is listed twice", file=sys.stderr)
>> > > > > > > + print("Symbol %s is listed twice" % line ,file=sys.stderr)
>> > > > > >
>> > > > > > Not a deal breaker, but IMO should at least the "new" syntax for string
>> > > > > > formatting using the .format() method (works both with python 2 and 3).
>> > > > > >
>> > > > > > Ideally, we'd move to python 3.6+ (since 2 will die in about 2 months) and
>> > > > > > started using string interpolation (or f-strings if you want).
>> > > > >
>> > > > > Well, looks like we are not using that anywhere. And frankly, f-strings are
>> > > > > horrible. This is the most readable style for us, C developers IMO.
>> > > >
>> > > > Can you be more specific on what exactly is horrible about f-strings? IMO it's
>> > > > actually very intuitive way of formatting strings unlike using the '%'
>> > > > formatting sign where depending on whether you have 1 or multiple arguments you
>> > > > may or may not need to use a tuple. F-strings are also a bit faster than the
>> > > > other formatting methods and because they're evaluated during runtime, you can
>> > > > evaluate arbitrary expressions, even call functions.
>> > >
>> > > That's exactly what I find horrible. Just consider the following example:
>> > >
>> > > print(f'a={f(x,n):d}, b={g(x,n):d}')
>> > >
>> > > IMO the following is more readable:
>> > >
>> > > print("a=%d, b=%d" % (f(x,n), g(x,n)))
>> > >
>> > > Once again, I'm talking about C developers (me specifically). I don't doubt
>> > > that an experienced python developer finds f-strings a step forward.
>> >
>> > I've plenty of python dev experiance and I still think f-strings are
>> > a horrible regression in design. IMHO it is a good thing to have the
>> > parameters visually separated from the string, as it makes it clearer
>> > to read the string being formatted.
>>
>> I tend to agree here. f-strings are not that bad but it allows you to
>> write ugly code. It is probably good for interactive python but
>> otherwise I would prefer the new-style .format() with named arguments:
>>
>> "Hello {name}".format(name="Pavel")
>>
>> The major benefit that I see in this style is that it helps translators
>> to figure out what the substitution will be and the order of arguments
>> doesn't matter and you can build the dictionary passed to format()
>> outside of it's call.
>
>Note those advantages are already possible with traditional syntax
>
> "Hello %(name)s" % { name: "Pavel"}
>
>but for our build scripts this is academic as we're never going to
>do translation of anything in them.
>
The advantage of "".format() is that you cannot make one mistake that can happen
with the currently used version and that is that sometimes you might need to
make sure the variable is in a tuple, i.e. difference between `params` and
`(params,)`. Even though this is *very* corner case, you still don't know if
the word after `%` is the only parameter or the iterable of parameters to be
used. Even though this is not *that* corner case, the mistakes that would cause
this would probably be caught quickly and auxiliary build scripts are not
something that needs to care about translations.
Since the biggest bikeshedding^Wconversations usually revolve around the
smallest patches, I felt like expressing myself as well =)
But anyway, even though "%d" is more C-like, when talking about the parameters, the `.format()` is more C-like IMHO. More importantly it should be the same in all of the code, so that it is consistent. And *most* importantly!!! It is just a auxiliary build script, just make sure it works and that's it =D
Have a nice day (with my preferred colour of the bikeshed),
Martin
P.S.: Just to illustrate the first mentioned difference, try reading the
following code. I think the `.format` makes it more understandable:
def a(*args):
print("{}".format(args))
def b(*args):
print("{}".format(*args))
def c(*args):
print("%r" % (args,))
def d(*args):
print("%r" % (*args,))
def e(*args):
print("%r" % args)
a('asdf')
b('asdf')
c('asdf')
d('asdf')
e('asdf')
a('asdf', 'fdsa')
b('asdf', 'fdsa')
c('asdf', 'fdsa')
#d('asdf', 'fdsa') # it easy to see why this fails, but the code is gross
#e('asdf', 'fdsa') # it is not easy to see why this fails
a()
#b() # it is easy to see why this fails
c()
#d() # it easy to see why this fails, but the code is gross
#e() # it is not easy to see why this fails
>Regards,
>Daniel
>--
>|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org -o- https://fstop138.berrange.com :|
>|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Nov 28, 2019 at 11:43:01AM +0100, Martin Kletzander wrote: >The advantage of "".format() is that you cannot make one mistake that can happen >with the currently used version and that is that sometimes you might need to >make sure the variable is in a tuple, i.e. difference between `params` and >`(params,)`. Even though this is *very* corner case, you still don't know if >the word after `%` is the only parameter or the iterable of parameters to be >used. Even though this is not *that* corner case, the mistakes that would cause >this would probably be caught quickly and auxiliary build scripts are not >something that needs to care about translations. > >Since the biggest bikeshedding^Wconversations usually revolve around the >smallest patches, I felt like expressing myself as well =) > >But anyway, even though "%d" is more C-like, when talking about the parameters, the `.format()` is more C-like IMHO. More importantly it should be the same in all of the code, so that it is consistent. And *most* importantly!!! It is just a auxiliary build script, just make sure it works and that's it =D *an auxiliary > >Have a nice day (with my preferred colour of the bikeshed), >Martin Best wishes, Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Michal Privoznik <mprivozn@redhat.com> [2019-11-26, 09:48AM +0100]:
> That's exactly what I find horrible. Just consider the following example:
>
> print(f'a={f(x,n):d}, b={g(x,n):d}')
>
> IMO the following is more readable:
>
> print("a=%d, b=%d" % (f(x,n), g(x,n)))
First of all, the format specifiers (i.e, ":d") are completely optional
if you just care about the string representation of an object.
Secondly, I certainly hope that such code never passes code review
anywhere. Properly cleaning this up to
a = f(x,n)
b = g(x,n)
print(f"a={a}, b={b}")
is much more readable then the old-style
print("a=%d, b=%d" % (a, b))
or even the .format-style
print("a={}, b={}".format(a, b))
Also, format strings have the best performance for string interpolation
in python which is a non-bikeshed argument for them.
Bjoern
--
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Nov 25, 2019 at 4:41 PM Peter Krempa <pkrempa@redhat.com> wrote:
>
> Commit d30a1ad0443 translated the symbol file checker from perl to
> python by doing a literal translation in most cases. Unfortunately one
> string formatting operation was not really translated into python
> leaving users with non-helpful error:
>
> 'Symbol $1 is listed twice'
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> scripts/check-symfile.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
> index 0c02591991..34396b8623 100755
> --- a/scripts/check-symfile.py
> +++ b/scripts/check-symfile.py
> @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
> line = line.strip(";")
>
> if line in wantsyms:
> - print("Symbol $1 is listed twice", file=sys.stderr)
> + print("Symbol %s is listed twice" % line ,file=sys.stderr)
Nitpick, 'line ,file=...' -> 'line, file=...'
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 26, 2019 at 09:51:18AM +0100, Fabiano Fidêncio wrote:
>On Mon, Nov 25, 2019 at 4:41 PM Peter Krempa <pkrempa@redhat.com> wrote:
>>
>> Commit d30a1ad0443 translated the symbol file checker from perl to
>> python by doing a literal translation in most cases. Unfortunately one
>> string formatting operation was not really translated into python
>> leaving users with non-helpful error:
>>
>> 'Symbol $1 is listed twice'
>>
>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> ---
>> scripts/check-symfile.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/check-symfile.py b/scripts/check-symfile.py
>> index 0c02591991..34396b8623 100755
>> --- a/scripts/check-symfile.py
>> +++ b/scripts/check-symfile.py
>> @@ -52,7 +52,7 @@ with open(symfile, "r") as fh:
>> line = line.strip(";")
>>
>> if line in wantsyms:
>> - print("Symbol $1 is listed twice", file=sys.stderr)
>> + print("Symbol %s is listed twice" % line ,file=sys.stderr)
>
>Nitpick, 'line ,file=...' -> 'line, file=...'
>
These are easily caught by syntax-check:
../scripts/check-symfile.py:55:53: E203 whitespace before ','
print("Symbol %s is listed twice" % line ,file=sys.stderr)
^
../scripts/check-symfile.py:55:54: E231 missing whitespace after ','
print("Symbol %s is listed twice" % line ,file=sys.stderr)
^
make: *** [../build-aux/syntax-check.mk:917: sc_flake8] Error 123
make: *** Waiting for unfinished jobs....
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.