[pep-8] Prefer “foo is None” to “foo == None”. Prefer “foo not in bar” to “not foo in bar”. #9581

1. 
   practicalswift commented at 9:13 am on January 19, 2017: contributor

   Prefer foo not in bar to not foo in bar.

   In accordance with Style Guide for Python Code (PEP 8):

   • https://www.python.org/dev/peps/pep-0008/#programming-recommendations
2. [pep-8] Prefer "foo not in bar" to "not foo in bar"

   In accordance with Style Guide for Python Code (PEP 8):
   * https://www.python.org/dev/peps/pep-0008/#programming-recommendations

   b806977734
3. 
   fanquake commented at 10:34 am on January 19, 2017: member

   @practicalswift See the pep-8 discussion in #6156. As well as the (clang-format) discussion in #6839 (similar to these kind of changes).

   If your going to do this, please combine all your changes into a single pull request. Then a more general discussion can happen about wether this is something we even want to do. As soon as you start half-enforcing style/formatting, you can just end up with “fix-up” pull-requests ever 2 weeks; while continuously breaking patches etc.

   Multiple single commit pull requests that only change formatting/styling/trivial things etc are not preferred, as each one requires review, and generates noise in an already pull-request flooded (with not enough people reviewing) repository.

   Contributions are always welcome, but please be mindful of the workload you could be creating for others (especially so close to a feature freeze). If possible, combine batches of smaller, related changes into single pull requests. If needed, a single important change can always be broken out, as already happens with the back-porting we do.

4. [pep-8] Prefer "foo is None" to "foo == None"

   In accordance with Style Guide for Python Code (PEP 8):
   * https://www.python.org/dev/peps/pep-0008/#programming-recommendations

   db68ddf24f
5. practicalswift renamed this:
   [pep-8] Prefer "foo not in bar" to "not foo in bar"
   [pep-8] Prefer "foo is None" to "foo == None". Prefer "foo not in bar" to "not foo in bar".
   on Jan 19, 2017
6. 
   practicalswift commented at 11:27 am on January 19, 2017: contributor

   @fanquake PRs now combined as requested.

   I totally agree that PEP-8 changes pertaining to whitespace, etc is overshooting.

   With regards to PEP-8 compliance my suggestion is to keep it to fixing only really obvious non-Pythonic things such as comparing to None, having unused imports, not being explicit about imports, etc - things that should be non-controversial, unlikely to cause any merge conflicts and unlikely to mess up git blame on a large scale.

   Basically what @MarcoFalke outlined in this comment:

   I think it makes sense to clean up the python code base in qa/ but please make sure you are not “exceeding the goal”. We already had cleanup pulls which changed to a max line length to 80 chars among other things. It turned out the patch was impossible to review, actually changed behavior and would have introduced bugs. I’d rather have “ugly” code that works than some PEP8 code that crashes.

   Sounds good? :-)

7. 
   MarcoFalke commented at 1:22 pm on January 19, 2017: member

   We should focus on stuff that is causing issues or is know to be likely to cause issues and not things that are merely coloring issues of the bike.

   For example there were a lot of issues in the python2 version of the rpc test, such as bare except: clauses with a pass in the body of the except clause, which makes the test pass regardless of what happens. This is obviously not something we want. Luckily, I fixed all of those when switching to python3.

8. 
   practicalswift commented at 4:40 pm on January 19, 2017: contributor

   @MarcoFalke Good points!

   These two changes specifically - avoiding foo == None and not foo in bar - are both of them to be considered in the “bikeshed area” in the context of this project? If so I’ll close this PR :-)

9. 
   practicalswift commented at 7:00 pm on January 29, 2017: contributor

   Is this one of interest or should I close it? :-)
10. practicalswift closed this on Jan 31, 2017

    ---

11. practicalswift deleted the branch on Apr 10, 2021
12. DrahtBot locked this on Aug 16, 2022

Contributors
practicalswift fanquake MarcoFalke