Code review comment for lp:~nahiljain/to-drizzle/parse_create_table

Hi!

Looks good, Nahil, but there's a few issues to fix up!

1) There is some extra print happening in the parse create table test case, as shown in the output here:

jpipes@serialcoder:~/repos/movetodrizzle/parse_create_table/tests$ ./parser_createtable_test.py
............[['AUTO_INCREMENT', 5]]
.
----------------------------------------------------------------------
Ran 13 tests in 0.009s

OK

Remove the printing of the [[AUTO_INCREMENT', 5]]

2) Remove all commented-out code:

60 +# '''foreign_key_constraint : FOREIGN KEY LPAREN index_col_name_list RPAREN reference_definition'''

174 +
175 +#def p_data_types(p):
176 +# '''data_type : mathematical'''
177 +# p[0]= p[1]
178 +
179 +
180 +

303 +#print parse_string("INSERT INTO neh1 VALUES (123,456,'abc');")
304 +#print parse_string("INSERT INTO neh1 SET a1=10,b2=25;")

491 +# constraint_name= False) (index_name=None) (parent_fields= ['field1']) (reference= Reference(name= reftable) (col_name_list= ['ref_field1']) (match_option= False) (on_delete= False) (on_update= False))])]

3) Please, please be consistent in your code and spacing. :)

134 + if(len(p)>3):
135 + p[0]=p[3]

Please put a single space between if and the (

Please put a single space before and after the >

Please put a single space after the =

There are more places where the style is inconsistent. Please do fix them up :)

4) Tabs and spacing is off in your code. For instance:

188 - '''temporary : TEMPORARY
189 + '''temporary : TEMPORARY
190 | empty'''
191 - # Returns True if the TEMPORARY phrase appeared, False otherwise
192 - p[0]= len(p) > 1
193 + # Returns True if the TEMPORARY phrase appeared, False otherwise
194 + p[0]= len(p) > 1

For some reason, you are re-setting Tabs to a single space. Please make sure all indentation is exactly 2 spaces.

5) Returned data_type should not be a list.

357 - self.assertEqual(str(ct.columns[0].data_type), "INT")
358 + self.assertEqual(str(ct.columns[0].data_type[0]), "INT")

Above, the columns[0].data_type should be "INT", not ["INT"] ...

Cheers!

jay