Mercurial > pylearn
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) + +