Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
On Thu, 13 Feb 2014 10:03:50 -0800 Alec Warner wrote: > On Thu, Feb 13, 2014 at 7:42 AM, Brian Dolbec > wrote: > > > well actually, for simple additions like that, string1 + string2, > > it is actually faster. > > But for multiple additions, %s is much better, faster. Also if the > > string is translated, then use %s regardless. That way the %s can > > be moved around for the translation. > > > > In general we prefer % for readability purposes, not because it is > faster. > > foo = "Bar" + foo + " " + baz + "," + goat > > foo = "Bar %s %s, %s" % (foo, baz, goat) > > I think this case could go either way, because even with %, > "NumberOf%s" is not much of an improvement. > The code is littered with the former though, and it makes it really > annoying to read ;) > > -A Do I have to sick Brian Harring on you ;) I said for simple string addition... string1 + string2 is faster and equally readable Your example goes into the "string %s %s, %s" example where the string substitution is actually faster. Plus it is a lot more readable. -- Brian Dolbec
Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
On Thu, Feb 13, 2014 at 7:42 AM, Brian Dolbec wrote:
> On Thu, 13 Feb 2014 03:19:35 -0500
> Mike Frysinger wrote:
>
> > On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> > > This patch adds a --output-style option to repoman, which gives the
> > > user a choice of output formats for the repoman checks. Choices are
> > > "default" (current style) and "column" (a greppable format), but it
> > > should be easy to add more. Fixes bug 481584.
> >
> > i'd expect a proper structured output would make sense to include in
> > the default set. like JSON. just create a dict and send it to
> > json.dump().
>
> He is working on more changes to repoman and the output. So, if you
> can, Chris, then do it, add a json option.
>
>
> >
> > > v2: Fix docstring to be complete and in the standard format, make
> > > use of default choices in --output-style wrt comments by antarus
> > > and dol-sen
> >
> > erm, i thought the previous docstring was correct. it followed
> > PEP257 while this new one is like javadoc or something.
> >
>
> It is the existing format that has been around in portage for years.
> There is even a page for it:
>
> http://www.gentoo.org/proj/en/portage/doc/policies/docstring-spec.xml
>
> It is also the style that epydoc recognizes.
>
> > > -utilities.format_qa_output(f, stats, fails, dofull, dofail,
> > > options, qawarnings)
> > > +if options.output_style == 'column':
> > > + utilities.format_qa_output_column(f, stats, fails, dofull,
> > > dofail, options, qawarnings)
> > > +else:
> > > + utilities.format_qa_output(f, stats, fails, dofull,
> > > dofail, options, qawarnings)
> >
> > use a func pointer instead.
> > format_outputs = {
> > 'column': utilities.format_qa_output_column,
> > 'default': utilities.format_qa_output,
> > }
> > format_output = format_outputs.get(options.output_style,
> > format_outputs['default'])
> > format_output(f, stats, fails, dofull, dofail, options, qawarnings)
> >
>
> yeah, make it so. Good spot, Mike
>
>
> Since Mike was too slow in replying, make another commit to change
> it.
>
> > > + formatter.add_literal_data("NumberOf " + category
> > > + " ")
> >
> > prefer to use % rather than + like so:
> > 'NumberOf %s ' % category
> >
> > > + formatter.add_literal_data("%s" % number)
> >
>
> well actually, for simple additions like that, string1 + string2, it is
> actually faster.
> But for multiple additions, %s is much better, faster. Also if the
> string is translated, then use %s regardless. That way the %s can be
> moved around for the translation.
>
In general we prefer % for readability purposes, not because it is faster.
foo = "Bar" + foo + " " + baz + "," + goat
foo = "Bar %s %s, %s" % (foo, baz, goat)
I think this case could go either way, because even with %, "NumberOf%s" is
not much of an improvement.
The code is littered with the former though, and it makes it really
annoying to read ;)
-A
>
> > str(number)
> > -mike
>
>
>
> --
> Brian Dolbec
>
>
Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
On Thu, Feb 13, 2014 at 12:19 AM, Mike Frysinger wrote:
> On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> > This patch adds a --output-style option to repoman, which gives the user
> > a choice of output formats for the repoman checks. Choices are "default"
> > (current style) and "column" (a greppable format), but it should be easy
> > to add more. Fixes bug 481584.
>
> i'd expect a proper structured output would make sense to include in the
> default set. like JSON. just create a dict and send it to json.dump().
>
He didn't want to, and I didn't want to make him ;)
>
> > v2: Fix docstring to be complete and in the standard format, make use of
> > default choices in --output-style wrt comments by antarus and dol-sen
>
> erm, i thought the previous docstring was correct. it followed PEP257
> while
> this new one is like javadoc or something.
>
> > -utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> > qawarnings)
> > +if options.output_style == 'column':
> > + utilities.format_qa_output_column(f, stats, fails, dofull, dofail,
> > options, qawarnings)
> > +else:
> > + utilities.format_qa_output(f, stats, fails, dofull, dofail,
> options,
> > qawarnings)
>
> use a func pointer instead.
> format_outputs = {
> 'column': utilities.format_qa_output_column,
> 'default': utilities.format_qa_output,
> }
> format_output = format_outputs.get(options.output_style,
> format_outputs['default'])
> format_output(f, stats, fails, dofull, dofail, options, qawarnings)
>
> > + formatter.add_literal_data("NumberOf " + category + " ")
>
> prefer to use % rather than + like so:
> 'NumberOf %s ' % category
>
> > + formatter.add_literal_data("%s" % number)
>
> str(number)
>
That is the definition of %s, no explicit str() is necessary.
http://docs.python.org/2/library/stdtypes.html#string-formatting-operations
> -mike
Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
On Thu, 13 Feb 2014 03:19:35 -0500
Mike Frysinger wrote:
> On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> > This patch adds a --output-style option to repoman, which gives the
> > user a choice of output formats for the repoman checks. Choices are
> > "default" (current style) and "column" (a greppable format), but it
> > should be easy to add more. Fixes bug 481584.
>
> i'd expect a proper structured output would make sense to include in
> the default set. like JSON. just create a dict and send it to
> json.dump().
He is working on more changes to repoman and the output. So, if you
can, Chris, then do it, add a json option.
>
> > v2: Fix docstring to be complete and in the standard format, make
> > use of default choices in --output-style wrt comments by antarus
> > and dol-sen
>
> erm, i thought the previous docstring was correct. it followed
> PEP257 while this new one is like javadoc or something.
>
It is the existing format that has been around in portage for years.
There is even a page for it:
http://www.gentoo.org/proj/en/portage/doc/policies/docstring-spec.xml
It is also the style that epydoc recognizes.
> > -utilities.format_qa_output(f, stats, fails, dofull, dofail,
> > options, qawarnings)
> > +if options.output_style == 'column':
> > + utilities.format_qa_output_column(f, stats, fails, dofull,
> > dofail, options, qawarnings)
> > +else:
> > + utilities.format_qa_output(f, stats, fails, dofull,
> > dofail, options, qawarnings)
>
> use a func pointer instead.
> format_outputs = {
> 'column': utilities.format_qa_output_column,
> 'default': utilities.format_qa_output,
> }
> format_output = format_outputs.get(options.output_style,
> format_outputs['default'])
> format_output(f, stats, fails, dofull, dofail, options, qawarnings)
>
yeah, make it so. Good spot, Mike
Since Mike was too slow in replying, make another commit to change
it.
> > + formatter.add_literal_data("NumberOf " + category
> > + " ")
>
> prefer to use % rather than + like so:
> 'NumberOf %s ' % category
>
> > + formatter.add_literal_data("%s" % number)
>
well actually, for simple additions like that, string1 + string2, it is
actually faster.
But for multiple additions, %s is much better, faster. Also if the
string is translated, then use %s regardless. That way the %s can be
moved around for the translation.
> str(number)
> -mike
--
Brian Dolbec
signature.asc
Description: PGP signature
Re: [gentoo-portage-dev] [PATCH v2] Add --output-style option to repoman
On Monday, February 10, 2014 20:22:36 Chris Reffett wrote:
> This patch adds a --output-style option to repoman, which gives the user
> a choice of output formats for the repoman checks. Choices are "default"
> (current style) and "column" (a greppable format), but it should be easy
> to add more. Fixes bug 481584.
i'd expect a proper structured output would make sense to include in the
default set. like JSON. just create a dict and send it to json.dump().
> v2: Fix docstring to be complete and in the standard format, make use of
> default choices in --output-style wrt comments by antarus and dol-sen
erm, i thought the previous docstring was correct. it followed PEP257 while
this new one is like javadoc or something.
> -utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> qawarnings)
> +if options.output_style == 'column':
> + utilities.format_qa_output_column(f, stats, fails, dofull, dofail,
> options, qawarnings)
> +else:
> + utilities.format_qa_output(f, stats, fails, dofull, dofail, options,
> qawarnings)
use a func pointer instead.
format_outputs = {
'column': utilities.format_qa_output_column,
'default': utilities.format_qa_output,
}
format_output = format_outputs.get(options.output_style,
format_outputs['default'])
format_output(f, stats, fails, dofull, dofail, options, qawarnings)
> + formatter.add_literal_data("NumberOf " + category + " ")
prefer to use % rather than + like so:
'NumberOf %s ' % category
> + formatter.add_literal_data("%s" % number)
str(number)
-mike
signature.asc
Description: This is a digitally signed message part.
