changeset 1142:4f03a9a743dc

Merged
author Olivier Delalleau <delallea@iro>
date Thu, 16 Sep 2010 13:28:50 -0400
parents 6c79394b6b20 (diff) 9f0502f8c7a5 (current diff)
children fa1715e759e3
files
diffstat 1 files changed, 97 insertions(+), 53 deletions(-) [+]
line wrap: on
line diff
--- a/doc/v2_planning/coding_style.txt	Thu Sep 16 13:27:17 2010 -0400
+++ b/doc/v2_planning/coding_style.txt	Thu Sep 16 13:28:50 2010 -0400
@@ -11,59 +11,6 @@
 Open for public debate
 ----------------------
 
-   * Use imports for packages and modules only. I.e. avoid
-        from foo import *
-        from foo import Bar
-     OD: Overall I agree with this. However we probably want to allow some
-        exceptions, like:
-            from itertools import imap, izip
-        Also, some people may want to have shortcuts like
-            from theano import tensor as T
-        but I would prefer to forbid this. It is handy when trying stuff in
-        the interactive interpreter, but in real code it can easily get messy
-        when you want to copy / paste different pieces of code and they use
-        different conventions. Typing tensor.* is a bit longer, but a lot more
-        portable.
-     JB: I thought that these are nice:
-         - "from foo import Bar" 
-         - "from foo import Bar, Blah"
-        What's wrong with them?  They keep the code listing short and readable.
-        I would discourage these forms when symbols 'Bar' and 'Blah' are
-        ambiguous, in which case the parent module prefix serves to disambiguate
-        them in the code.
-        I agree that the "import A as B" form should be discouraged in general,
-        because that's just confusing and makes code less grep-friendly.
-     OD: I agree that "from foo import Bar, Blah" is sometimes convenient
-        (typically when you re-use Bar / Blah many times in the same file),
-        and would vote in favor of accepting it when it is appropriate.
-        This guideline was taken from Google's coding recommendation:
-            "from foo import * or from foo import Bar is very nasty and can
-             lead to serious maintenance issues because it makes it hard to find
-             module dependencies."
-
-   * Imports should usually be on separate lines.
-     OD: I would add an exception, saying it is ok to group multiple imports
-        from the standard library on a single line, e.g.
-            import os, sys, time
-        I just don't see much benefit in putting them on separate lines (for
-        third-party imports I agree it is best to keep them separate, as it
-        makes dependencies clearer, and diffs look better when someone adds /
-        removes an import).  Does anyone see a good reason to keep standard
-        library imports on different lines?
-     JB: what does 'usually' mean here? The guideline seems vacuous.
-     OD: Sorry my fault, I did not quote the whole guideline from PEP8. The
-         'usually' was because of what followed:
-            it's okay to say this though:
-                from subprocess import Popen, PIPE
-         (which btw contradicts Google's recommendation mentioned previously)
-
-    * The BDFL recommends inserting a blank line between the
-      last paragraph in a multi-line docstring and its closing quotes, placing
-      the closing quotes on a line by themselves. This way, Emacs'
-      fill-paragraph command can be used on it.
-      OD: I think it is ugly and I have not seen it used much. Any Emacs
-        user believes it is a must?
-
     * Avoid contractions in code comments (particularly in
       documentation): "We do not add blue to red because it does not look good"
       rather than "We don't add blue to red because it doesn't look good".
@@ -112,6 +59,70 @@
 
       JB: +0.5
 
+Closed for public debate
+------------------------
+
+   * Use imports for packages and modules only. I.e. avoid
+        from foo import *
+        from foo import Bar
+     OD: Overall I agree with this. However we probably want to allow some
+        exceptions, like:
+            from itertools import imap, izip
+        Also, some people may want to have shortcuts like
+            from theano import tensor as T
+        but I would prefer to forbid this. It is handy when trying stuff in
+        the interactive interpreter, but in real code it can easily get messy
+        when you want to copy / paste different pieces of code and they use
+        different conventions. Typing tensor.* is a bit longer, but a lot more
+        portable.
+     JB: I thought that these are nice:
+         - "from foo import Bar" 
+         - "from foo import Bar, Blah"
+        What's wrong with them?  They keep the code listing short and readable.
+        I would discourage these forms when symbols 'Bar' and 'Blah' are
+        ambiguous, in which case the parent module prefix serves to disambiguate
+        them in the code.
+        I agree that the "import A as B" form should be discouraged in general,
+        because that's just confusing and makes code less grep-friendly.
+     OD: I agree that "from foo import Bar, Blah" is sometimes convenient
+        (typically when you re-use Bar / Blah many times in the same file),
+        and would vote in favor of accepting it when it is appropriate.
+        This guideline was taken from Google's coding recommendation:
+            "from foo import * or from foo import Bar is very nasty and can
+             lead to serious maintenance issues because it makes it hard to find
+             module dependencies."
+     OD: Decision was taken in committee's meeting to allow
+            from foo import Bar, Blah
+         when imported stuff is re-used multiple times in the same file, and
+         there is no ambiguity.
+
+   * Imports should usually be on separate lines.
+     OD: I would add an exception, saying it is ok to group multiple imports
+        from the standard library on a single line, e.g.
+            import os, sys, time
+        I just don't see much benefit in putting them on separate lines (for
+        third-party imports I agree it is best to keep them separate, as it
+        makes dependencies clearer, and diffs look better when someone adds /
+        removes an import).  Does anyone see a good reason to keep standard
+        library imports on different lines?
+     JB: what does 'usually' mean here? The guideline seems vacuous.
+     OD: Sorry my fault, I did not quote the whole guideline from PEP8. The
+         'usually' was because of what followed:
+            it's okay to say this though:
+                from subprocess import Popen, PIPE
+         (which btw contradicts Google's recommendation mentioned previously)
+     OD: Decision was taken in committee's meeting to allow multiple imports
+         on the same line for standard library modules (only).
+
+    * The BDFL recommends inserting a blank line between the
+      last paragraph in a multi-line docstring and its closing quotes, placing
+      the closing quotes on a line by themselves. This way, Emacs'
+      fill-paragraph command can be used on it.
+      OD: I think it is ugly and I have not seen it used much. Any Emacs
+          user believes it is a must?
+      OD: Decision was taken in committee's meeting to drop this
+          recommendation.
+
     * JB: How should we combine capitalization and underscores to name classes
           and functions related to an algorithm like 'SGD' or a model like 'RBM'
           whose common name is capitalized?  Case in point: How should I name a
@@ -127,6 +138,7 @@
           so I guess HMC is ok when using Hybrid Monte Carlo is considered to
           make some names too long.
 
+
 Note about warnings
 -------------------
 
@@ -388,6 +400,8 @@
       define all and any.
       OD: I think we should have something like pylearn.compat.{all,any}.
           numpy.{all,any} are meant to be used on arrays only.
+      OD: As agreed during committee's meeting, we will use
+          theano.gof.python25
 
     * Do not use the `hashlib` module (not supported in Python 2.4).
       You can do as in theano.gof.cc:
@@ -403,6 +417,15 @@
     OD: Yep, we could probably come up with such a wrapper in a pylearn.compat
         module.
 
+    * Do not use mutable arguments as default values. Instead, use a helper
+      function:
+      Yes:
+        def f(array=None):
+            array = pylearn.if_none(array, [])
+      No:
+        def f(array=[]):
+            # Dangerous if `array` is modified down the road.
+
 Mercurial commits
 -----------------
 
@@ -410,6 +433,9 @@
     OD: Check Django's guidelines (link above)
    * Standardize the merge commit text (what is the message from fetch?)
 
+During committee's meeting, Fred mentioned a bug with Assembla links for
+multi-line commits.
+
 Type checking
 -------------
 
@@ -439,6 +465,9 @@
 vous pourriez p-e définir des type/value-checkers standards pour éviter que
 tout le monde redéfinissent les siens à sa façon.
 
+OD: This was discussed in committee's meeting. We agreed to provide ways to do
+this, but not to enforce its usage.
+
 Consistent inf / nan
 --------------------
 
@@ -446,6 +475,7 @@
 (should be slightly more efficient even if efficiency usually doesn't matter
 here - the main goal would be for everyone to use the same inf / nan to make
 the code consistent).
+OD: Approved during committee's meeting.
 
 Enforcing strict testing policy
 -------------------------------
@@ -477,3 +507,17 @@
 - if there is no warning, daily email notification (ADD A WARNING!!!)
 - if there is a warning, weekly email notification (ADD A TEST!!!)
 
+Meeting 2010/09/16
+------------------
+
+Tasks to be performed by tomorrow:
+    * OD:
+        * Write down summary of Python coding style recommendations
+        * Start a file that showcases those guidelines
+    * DWF:
+        * Look into recommendations on how to document a class, method, ...
+        * Write recommendations on when to use logging vs. warning
+        * Make public some configuration files / plugins for vim
+        * Come up with official common file header (license in particular)
+
+