Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2018-02-23 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:
 |  Someday/Maybe
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * status:  new => closed
 * resolution:   => wontfix


Comment:

 The patch has gone stale and the discussion on the PR didn't yield a
 strong consensus. I think the value of trying to enforce some guidelines
 around this style is minimal.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.1c9dd09943a3fc7cb3134fe24095fbe2%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-12-27 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Someday/Maybe
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * stage:  Unreviewed => Someday/Maybe


Comment:

 The reporter already provided a patch and I created a
 [https://github.com/django/django/pull/9498 PR]. Opinions welcome.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.dacd03b346f4cb83319bc9cfa4c1e4fa%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-12-04 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Adam (Chainz) Johnson):

 So I'm very slightly in favour of removing the useless 'elses', I also
 consider it a tiny bit more readable, and amazingly cpython *does*
 generate less code in this case (it's no faster though, it's just removing
 an extra two opcodes from a redundant return None):

 {{{
 In [6]: def foo():
...: if foo:
...: return 1
...: else:
...: return 2
...:

 In [7]: def bar():
...: if bar:
...: return 1
...: return 2
...:

 In [8]: dis.dis(foo)
   2   0 LOAD_GLOBAL  0 (foo)
   2 POP_JUMP_IF_FALSE8

   3   4 LOAD_CONST   1 (1)
   6 RETURN_VALUE

   5 >>8 LOAD_CONST   2 (2)
  10 RETURN_VALUE
  12 LOAD_CONST   0 (None)
  14 RETURN_VALUE

 In [9]: dis.dis(bar)
   2   0 LOAD_GLOBAL  0 (bar)
   2 POP_JUMP_IF_FALSE8

   3   4 LOAD_CONST   1 (1)
   6 RETURN_VALUE

   4 >>8 LOAD_CONST   2 (2)
  10 RETURN_VALUE
 }}}

 However I doubt it's worth anyone's time to go through Django removing
 them. Perhaps we can add it to the style guide now, close this ticket, and
 fix it as code gets modified in the future.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.b1db93c286eeef04c06332632a934bb9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-12-03 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Дилян Палаузов):

 python has no switch..case, but "elif", which is not the same.  The
 readability criterion is very subjective one, e.g. the author might not
 have been aware that the 'else' was useless. As for
 {{{
 if x:
 return y
 else:
 raise z
 }}}
 the {{{else}}} does not add for readability.

 The first snippets of django/contrib/gis/gdal/srs.py or
 django/contrib/gis/geoip2/base.py I sent, have nothing to do with
 readability.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.86ddcea5dcca6b218b606933179266c6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-12-03 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Tomer Chachamu):

 I think you answered your own question there - it is used when it impacts
 readability (as judged by the original authors, who may not always agree
 with each other).

 An `if..elif..else..` block clearly lists mutually exclusive branches
 taken, and can be seen by looking at a single indentation level.
 `if..return; if.. return;..` requires checking two indentation levels to
 ensure there's a `return/raise` ending every `if`.

 For example, in the first hunk of the patch, the code is:

 {{{
 if bla:
some code
 if bla2:
raise
 elif bla3:
raise
 return val
 }}}

 After the change, it is:

 {{{
 if bla:
some code
 if bla2:
raise
 if bla3:
raise
 return val
 }}}

 A newline before `if bla2` would improve readability, however the `elif`
 is already helping to show there is some similarity between `if bla2` and
 `if bla3`, and that `if bla` has a different purpose.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.4a3e69c069c6393cf76a3564c2b2cdbd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-12-03 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Дилян Палаузов):

 Skipping {{{else}}} after {{{return}}}, when it does not impact
 readability, leads to less code, and the resulting code is faster to read
 (by humans).

 Currently it is unclear, when useless {{{else}}} shall be written.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.5ce387d81c956356aec0f248228f8d98%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-12-02 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tomer Chachamu):

 * cc: Tomer Chachamu (added)


Comment:

 The generated CPython bytecode is usually the same, I see no benefit in
 applying this rule.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.b38b9db64991a82964144c64a08dfa3f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-11-29 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * component:  Uncategorized => Core (Other)


Comment:

 Personally, I think it has some readability benefits, at least in some
 places.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.b2eda4902ea670f44a2d26ee3335ddd8%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.


Re: [Django] #28858: Remove 'else' after 'return' or 'raise'

2017-11-29 Thread Django
#28858: Remove 'else' after 'return' or 'raise'
-+-
 Reporter:  Дилян Палаузов   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Uncategorized|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Дилян Палаузов):

 * Attachment "no-else-after-return.patch" added.


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.f874f21766bb71ea22e1f7ef535f8fe0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.