Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

Trevor Wennblom
i've been developing within Galaxy for awhile now and have been enjoying it quite a bit. i've seen portions of similar ideas in the past, but never cohesively assembled and so well realized.

i'd like to make some suggestions for further code collaboration / community contributions.

my default development environment seems to occasionally be causing havoc within my local repository. upon saving, i normally automatically strip right-sided whitespace to stay compatible with other developers i collaborate with. this also fixes the auto-indentation left by my editor when it occasionally leaves a line consisting of only spaces.

given that python has syntactically significant whitespace, i also try to maintain the convention of indentation with four-spaces. i've noticed this isn't consistent within the codebase, but does seem to be the preferred style such as in `lib/galaxy/datatypes/`.

python comes packaged with the script `reindent.py`:

    Change Python (.py) files to use 4-space indents and no hard tab characters.
    Also trim excess spaces and tabs from ends of lines, and remove empty lines
    at the end of files.  Also ensure the last line ends with a newline.


this is recommended practice per PEP 8:
  http://www.python.org/dev/peps/pep-0008/

  Tabs or Spaces?

      Never mix tabs and spaces.

      The most popular way of indenting Python is with spaces only.  The
      second-most popular way is with tabs only.  Code indented with a mixture
      of tabs and spaces should be converted to using spaces exclusively.  When
      invoking the Python command line interpreter with the -t option, it issues
      warnings about code that illegally mixes tabs and spaces.  When using -tt
      these warnings become errors.  These options are highly recommended!

      For new projects, spaces-only are strongly recommended over tabs.  Most
      editors have features that make this easy to do.


other comments in regards to collaboration:
  http://rails-bestpractices.com/posts/60-remove-trailing-whitespace
  http://blogobaggins.com/2009/03/31/waging-war-on-whitespace.html


hooks can be added to the repository for consistency, as i'm sure with the many contributions received this would be easy to miss:
  http://mercurial.selenic.com/wiki/CheckFilesExtension


would anyone be opposed to me fixing up the current codebase to adhere to this? running `reindent.py` on the files is easy enough, i'm willing to step through the files (`opendiff` / `FileMerge.app`) and verify no unlikely syntactic changes have occurred. i can also deliver changes in gradual "chunked" pull requests to ease current developers getting possibly bit by merge issues.

would anyone be willing to add the appropriate hooks to the central repository as well?


trevor
[hidden email]
https://github.com/trevor
___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

Peter Cock
On Thu, Sep 1, 2011 at 9:00 PM, Trevor Wennblom <[hidden email]> wrote:

> ...
> given that python has syntactically significant whitespace, i also
> try to maintain the convention of indentation with four-spaces.
> i've noticed this isn't consistent within the codebase, but does
>  seem to be the preferred style such as in `lib/galaxy/datatypes/`.
>
> python comes packaged with the script `reindent.py`:
>
> ...
>
> this is recommended practice per PEP 8:
>  http://www.python.org/dev/peps/pep-0008/
>
> ...
>
> would anyone be opposed to me fixing up the current codebase
> to adhere to this? running `reindent.py` on the files is easy enough,
> i'm willing to step through the files (`opendiff` / `FileMerge.app`)
> and verify no unlikely syntactic changes have occurred. i can also
> deliver changes in gradual "chunked" pull requests to ease
> current developers getting possibly bit by merge issues.

+1 on correcting any tabs to spaces in the Galaxy Python code.
Doing this in chunked commits makes good sense too - although
if you can get one of the Galaxy team to do this directly it might
be quicker.

Personally I'd like to go further and fix the non-PEP8 white
space in most of the Galaxy Python code, e.g.

function ( argument )

rather than:

function(argument).

Chatting to some of the Galaxy team at BOSC/ISMB 2011
there is some support for this internally. Again, there are
automated tools to do this.

> would anyone be willing to add the appropriate hooks to
> the central repository as well?

As long as there are no false positives identified during
the initial tab/space conversion that seems sensible to
prevent new tabs creeping in. But not essential.

Peter

___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

Nate Coraor (nate@bx.psu.edu)
Peter Cock wrote:

> On Thu, Sep 1, 2011 at 9:00 PM, Trevor Wennblom <[hidden email]> wrote:
> > ...
> > given that python has syntactically significant whitespace, i also
> > try to maintain the convention of indentation with four-spaces.
> > i've noticed this isn't consistent within the codebase, but does
> >  seem to be the preferred style such as in `lib/galaxy/datatypes/`.
> >
> > python comes packaged with the script `reindent.py`:
> >
> > ...
> >
> > this is recommended practice per PEP 8:
> >  http://www.python.org/dev/peps/pep-0008/
> >
> > ...
> >
> > would anyone be opposed to me fixing up the current codebase
> > to adhere to this? running `reindent.py` on the files is easy enough,
> > i'm willing to step through the files (`opendiff` / `FileMerge.app`)
> > and verify no unlikely syntactic changes have occurred. i can also
> > deliver changes in gradual "chunked" pull requests to ease
> > current developers getting possibly bit by merge issues.
>
> +1 on correcting any tabs to spaces in the Galaxy Python code.
> Doing this in chunked commits makes good sense too - although
> if you can get one of the Galaxy team to do this directly it might
> be quicker.

It is the Galaxy Team's intent to use four-space indents, anything else
is a mistake and we try to fix 'em as we see 'em.  Nobody has yet taken
the time to fix them all (i.e. with reindent.py) but such fixes would be
welcome.

> Personally I'd like to go further and fix the non-PEP8 white
> space in most of the Galaxy Python code, e.g.
>
> function ( argument )
>
> rather than:
>
> function(argument).

There are a lot of parts of PEP 8 that I doubt we want to adhere to
strictly.  I agree it's annoying to find varying styles, but the space
inside parentheses is not one I get too worked up about.

FWIW, I prefer function( argument ) and I tend to see that style out of
most of the rest of the team.

> Chatting to some of the Galaxy team at BOSC/ISMB 2011
> there is some support for this internally. Again, there are
> automated tools to do this.
>
> > would anyone be willing to add the appropriate hooks to
> > the central repository as well?
>
> As long as there are no false positives identified during
> the initial tab/space conversion that seems sensible to
> prevent new tabs creeping in. But not essential.

We can't add custom hooks to the bitbucket repository and we don't have
an intermediate local repository that we all push changesets through.

--nate

>
> Peter
>
> ___________________________________________________________
> Please keep all replies on the list by using "reply all"
> in your mail client.  To manage your subscriptions to this
> and other Galaxy lists, please use the interface at:
>
>   http://lists.bx.psu.edu/
>
___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

Fields, Christopher J
On Sep 2, 2011, at 9:57 AM, Nate Coraor wrote:

> ...
>> Chatting to some of the Galaxy team at BOSC/ISMB 2011
>> there is some support for this internally. Again, there are
>> automated tools to do this.
>>
>>> would anyone be willing to add the appropriate hooks to
>>> the central repository as well?
>>
>> As long as there are no false positives identified during
>> the initial tab/space conversion that seems sensible to
>> prevent new tabs creeping in. But not essential.
>
> We can't add custom hooks to the bitbucket repository and we don't have
> an intermediate local repository that we all push changesets through.
>
> --nate

The same applies for most public repos like bitbucket; github is the same (only post-receive is allowed there).  I'm unsure whether repository hooks are stored on github, but they are definitely ignored (only specific post-receive hooks are allowed, and these are set up via the github admin API).

With git the only way I can think of to set something like this up is client-side (pre/post-commit, or pre-push), then maybe have a separate post-checkout hook to set everything up after a clone.  I assume hg has a similar mechanism.

chris


___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

Nate Coraor (nate@bx.psu.edu)
Fields, Christopher J wrote:

> On Sep 2, 2011, at 9:57 AM, Nate Coraor wrote:
>
> > ...
> >> Chatting to some of the Galaxy team at BOSC/ISMB 2011
> >> there is some support for this internally. Again, there are
> >> automated tools to do this.
> >>
> >>> would anyone be willing to add the appropriate hooks to
> >>> the central repository as well?
> >>
> >> As long as there are no false positives identified during
> >> the initial tab/space conversion that seems sensible to
> >> prevent new tabs creeping in. But not essential.
> >
> > We can't add custom hooks to the bitbucket repository and we don't have
> > an intermediate local repository that we all push changesets through.
> >
> > --nate
>
> The same applies for most public repos like bitbucket; github is the same (only post-receive is allowed there).  I'm unsure whether repository hooks are stored on github, but they are definitely ignored (only specific post-receive hooks are allowed, and these are set up via the github admin API).
>
> With git the only way I can think of to set something like this up is client-side (pre/post-commit, or pre-push), then maybe have a separate post-checkout hook to set everything up after a clone.  I assume hg has a similar mechanism.

Yeah, everyone could do it locally.

>
> chris
>
>
___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

Dave Clements-3
Hello all,

I've created a wiki page on coding best practices to record what is actually done, and the results of discussions like these:
   http://wiki.g2.bx.psu.edu/Develop/Best Practices

So far, it only lists 2 standards:

1. 4 spaces per indent level
2. Use spaces, not tabs.

I'll continue to watch this list and add best practices accordingly.

Dave C.

--
http://galaxyproject.org/
http://getgalaxy.org/
http://usegalaxy.org/
http://galaxyproject.org/wiki/


___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/
Reply | Threaded
Open this post in threaded view
|

Re: Suggestion / Request for Comments on Galaxy Best Practices - Gradual migration to standard indentation

James Taylor-2
Coding standards have been discussed before, but perhaps never written down. So, here is my interpretation:

We follow PEP-8, with particular emphasis on the parts about knowing when to be inconsistent, and readability being the ultimate goal. In addition, we make the following specific exceptions:

Line length: comments and documentation comments should follow the 79 character rule, other lines should be formatted for readability, recognizing the existence of wide screens and scrollbars (sometimes a 200 character line is more readable, though rarely).

Spacing: whitespace around operators and inside parentheses et al. is okay, particularly if it helps readability. We are widely inconsistent on this, if the thing inside the parentheses is short (a single word) often spaces are extraneous, if it is a complex expression, spacing helps to delineate different parts.

Blank lines: also subjective, the size of the class, number of methods, size of methods, et cetera all influence what sort of spacing is most readable.

This subjectivity makes it very hard to run a systematic check for spacing and line length deviations. The more obvious things like tabs used for indentation are bugs.

On Sep 2, 2011, at 4:54 PM, Dave Clements wrote:

> Hello all,
>
> I've created a wiki page on coding best practices to record what is actually done, and the results of discussions like these:
>    http://wiki.g2.bx.psu.edu/Develop/Best Practices
>
> So far, it only lists 2 standards:
>
> 1. 4 spaces per indent level
> 2. Use spaces, not tabs.
>
> I'll continue to watch this list and add best practices accordingly.
>
> Dave C.
>
> --
> http://galaxyproject.org/
> http://getgalaxy.org/
> http://usegalaxy.org/
> http://galaxyproject.org/wiki/
>
> ___________________________________________________________
> Please keep all replies on the list by using "reply all"
> in your mail client.  To manage your subscriptions to this
> and other Galaxy lists, please use the interface at:
>
>  http://lists.bx.psu.edu/


___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:

  http://lists.bx.psu.edu/