Page 1 of 1

aligncsv.py needs updating

Posted: 23 Aug 2011, 18:13
by o11c
  • The most severe problem is that this treats commas in {scripts} as if they were top-level commas.
  • The consensus has changed from tabs to spaces, both for the output of the script as well as the indentation of the script itself. Actually, even if the consensus had not changed, tabs should NEVER be used for alignment, only for indentation.
  • The destination file should not be required to exist.
  • Check whether the required arguments exist before doing anything else.
  • There should be a check for whether open(fname, "mode") succeeded, rather than just if the file exists.
  • Avoid the repeated calls to .strip()
  • General review by somebody who actually knows python. More comments would be helpful for those of us who don't (what's with all the [-1]).
When this is said and done, it can be made a policy to run this before committing CSVs. (TODO look at precommit hooks - IIRC it must be made a filter from stdin to stdout then).

Is it worth saving this or should it be rewritten from scratch?

Re: aligncsv.py needs updating

Posted: 23 Aug 2011, 20:37
by argul
o11c wrote:
  • General review by somebody who actually knows python. More comments would be helpful for those of us who don't (what's with all the [-1]).
When this is said and done, it can be made a policy to run this before committing CSVs. (TODO look at precommit hooks - IIRC it must be made a filter from stdin to stdout then).

Is it worth saving this or should it be rewritten from scratch?
Originally I wrote that script.
And yes it's a so-called 'hack', so I don't mind if anyone rewrites that script in any language you like to.
hooks as you said :-D nice ones:)

Iirc, the script supports both indentation with tabs and spaces, not sure if they are on my local disc or already in the git repos.

In python you can address arrays from either left to right (so index is 0,1,2,3) or from right to left (negative numbers, so -1,-2,-3 are indices seen from right)

Re: aligncsv.py needs updating

Posted: 16 Sep 2011, 21:59
by o11c
Done, in C++, in commit ed81f91ac3c470e00df0d5c5cf9c274ce30f47b3