Issue 5844 - Remove redundant parentheses in sitescripts and abpsembly

Remove redundant parentheses flagged by the new flake8 rule. I tested each modification against the original using pydiff, and the resulting bytecode is identical.

[Sebastian Noack, 2018-01-17 01:43:52]

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:155: name, url = (self.name,
value)
I wonder whether we should omit the parantheses when creating the tuple as well,
here. The rule allows the optional parantheses when creating tuples (for
readability), but not when unpacking tuples. However, if the same line does
both, it looks somewhat inconsistent.

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/templat...
File sitescripts/templateFilters.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/templat...
sitescripts/templateFilters.py:132: )
If you indent code without a newline after the opening parenthesis, please don't
put the closing parenthesis on a new line either:

  value.sort(lambda a, b:
             ...
             ...)

Alternatively, following indention would be acceptable, and compliant with
flake8, too, and while it requires 2 more lines it looks a bit more structured:

  value.sort(
      lambda a, b:
          ...
  )

FWIW, one could also argue that neither is an improvement to the original code.
However, if we just remove the parenthesis, without changing the indentation,
we'd get an E128. Not sure if it's worth to add a (potentially complex)
exception to the A111 rule, though. What would the exception be, anyway?

[Vasily Kuznetsov, 2018-01-18 10:46:07]

Hi Rosie!

In general everything looks good. I just have a couple of comments on
Sebastian's comments (generally I agree with him).

Cheers,
Vasily

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:155: name, url = (self.name,
value)
On 2018/01/17 01:43:51, Sebastian Noack wrote:
> I wonder whether we should omit the parantheses when creating the tuple as
well,
> here. The rule allows the optional parantheses when creating tuples (for
> readability), but not when unpacking tuples. However, if the same line does
> both, it looks somewhat inconsistent.

If parentheses are not needed for line breaking (and they should not be in
constructions like this), my preference would be to omit parentheses on both
sides. More consistent and reads a bit better. I don't feel very strong about
this though.

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/templat...
File sitescripts/templateFilters.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/templat...
sitescripts/templateFilters.py:132: )
On 2018/01/17 01:43:51, Sebastian Noack wrote:
> If you indent code without a newline after the opening parenthesis, please
don't
> put the closing parenthesis on a new line either:
> 
>   value.sort(lambda a, b:
>              ...
>              ...)
> 
> Alternatively, following indention would be acceptable, and compliant with
> flake8, too, and while it requires 2 more lines it looks a bit more
structured:
> 
>   value.sort(
>       lambda a, b:
>           ...
>   )
> 
> FWIW, one could also argue that neither is an improvement to the original
code.
> However, if we just remove the parenthesis, without changing the indentation,
> we'd get an E128. Not sure if it's worth to add a (potentially complex)
> exception to the A111 rule, though. What would the exception be, anyway?

Just for the record, I prefer Sebastian's second version. I also think it's more
readable than the original code.

[rosie, 2018-01-25 16:13:48]

Hi Vasily and Sebastian!

I made the two changes that were suggested. Hope everything else is ok.  :)

Best,
Rosie

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:155: name, url = (self.name,
value)
On 2018/01/18 10:46:07, Vasily Kuznetsov wrote:
> On 2018/01/17 01:43:51, Sebastian Noack wrote:
> > I wonder whether we should omit the parantheses when creating the tuple as
> well,
> > here. The rule allows the optional parantheses when creating tuples (for
> > readability), but not when unpacking tuples. However, if the same line does
> > both, it looks somewhat inconsistent.
> 
> If parentheses are not needed for line breaking (and they should not be in
> constructions like this), my preference would be to omit parentheses on both
> sides. More consistent and reads a bit better. I don't feel very strong about
> this though.

Done.

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/templat...
File sitescripts/templateFilters.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/templat...
sitescripts/templateFilters.py:132: )
On 2018/01/18 10:46:07, Vasily Kuznetsov wrote:
> On 2018/01/17 01:43:51, Sebastian Noack wrote:
> > If you indent code without a newline after the opening parenthesis, please
> don't
> > put the closing parenthesis on a new line either:
> > 
> >   value.sort(lambda a, b:
> >              ...
> >              ...)
> > 
> > Alternatively, following indention would be acceptable, and compliant with
> > flake8, too, and while it requires 2 more lines it looks a bit more
> structured:
> > 
> >   value.sort(
> >       lambda a, b:
> >           ...
> >   )
> > 
> > FWIW, one could also argue that neither is an improvement to the original
> code.
> > However, if we just remove the parenthesis, without changing the
indentation,
> > we'd get an E128. Not sure if it's worth to add a (potentially complex)
> > exception to the A111 rule, though. What would the exception be, anyway?
> 
> Just for the record, I prefer Sebastian's second version. I also think it's
more
> readable than the original code.

Yeah, the second version also seems more readable to me.
Done.

[Sebastian Noack, 2018-01-25 18:15:01]

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:155: name, url = (self.name,
value)
On 2018/01/25 16:13:48, rosie wrote:
> On 2018/01/18 10:46:07, Vasily Kuznetsov wrote:
> > On 2018/01/17 01:43:51, Sebastian Noack wrote:
> > > I wonder whether we should omit the parantheses when creating the tuple as
> > well,
> > > here. The rule allows the optional parantheses when creating tuples (for
> > > readability), but not when unpacking tuples. However, if the same line
does
> > > both, it looks somewhat inconsistent.
> > 
> > If parentheses are not needed for line breaking (and they should not be in
> > constructions like this), my preference would be to omit parentheses on both
> > sides. More consistent and reads a bit better. I don't feel very strong
about
> > this though.
> 
> Done.

Mind also doing the same change below?

[rosie, 2018-01-25 19:03:40]

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
File sitescripts/subscriptions/subscriptionParser.py (right):

https://codereview.adblockplus.org/29670611/diff/29670612/sitescripts/subscri...
sitescripts/subscriptions/subscriptionParser.py:155: name, url = (self.name,
value)
On 2018/01/25 18:15:00, Sebastian Noack wrote:
> On 2018/01/25 16:13:48, rosie wrote:
> > On 2018/01/18 10:46:07, Vasily Kuznetsov wrote:
> > > On 2018/01/17 01:43:51, Sebastian Noack wrote:
> > > > I wonder whether we should omit the parantheses when creating the tuple
as
> > > well,
> > > > here. The rule allows the optional parantheses when creating tuples (for
> > > > readability), but not when unpacking tuples. However, if the same line
> does
> > > > both, it looks somewhat inconsistent.
> > > 
> > > If parentheses are not needed for line breaking (and they should not be in
> > > constructions like this), my preference would be to omit parentheses on
both
> > > sides. More consistent and reads a bit better. I don't feel very strong
> about
> > > this though.
> > 
> > Done.
> 
> Mind also doing the same change below?

Done.

[Sebastian Noack, 2018-01-25 19:24:00]

LGTM. Can you please close the review for apbsembly, and update the title of
this review to reflect that this patch should be merged into both repositories.
I confirmed that the patch for abpsembly is identical to this one except for the
comments addressed.

[rosie, 2018-01-25 19:30:44]

On 2018/01/25 19:24:00, Sebastian Noack wrote:
> LGTM. Can you please close the review for apbsembly, and update the title of
> this review to reflect that this patch should be merged into both
repositories.
> I confirmed that the patch for abpsembly is identical to this one except for
the
> comments addressed.

Done.

[Vasily Kuznetsov, 2018-01-25 19:40:48]

LGTM