[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[task #15317] Concatenate two or more tables

From: Mohammad Akhlaghi
Subject: [task #15317] Concatenate two or more tables
Date: Mon, 16 Mar 2020 17:49:47 -0400 (EDT)
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:74.0) Gecko/20100101 Firefox/74.0

Follow-up Comment #15, task #15317 (project gnuastro):

Thanks, its getting much better ;-). I can do all of these myself (and it
would take much less time for me than writing these comments), but I think it
is much better to get you to a good start now. This way, your future projects
in Gnuastro will be much more smoothly merged :-D.

Here are some points that occurred to me:

0 The commit message description is very good now, thanks! But length of the
lines in the commit message make it very hard to read. Take a look at some
previous commit messages: all the lines have roughly the same length. Also,
the last one is too long (it shouldn't be more than 75 characters).
0 The name of `concat_along_row' doesn't follow the standard and is confusing:
since its in a file called `table.c' it should start with `table_'. Also,
since its job is concatenate columns after each other, having the name `row'
can be confusing. Its good to give it a very similar name to the option.
0 In `concat_along_row', you can use the `gal_list_data_last' function (in
to avoid the while loop and help in readability.
0 The function and loop braces (`{' and `}') don't follow the GNU style
<>. Please have a
look at the other functions/loops to get a good feeling ;-).
0 Since this option is about `c'olumns, maybe setting `UI_KEY_CATCOLUMN' to
`C' is a better and more clear choice for `--catcolumn' compared to `f', what
do you think?
0 You can just define `catcolumntable' in `concat_along_row', not in the main
program structure. You can simply pass it the name of the file, and it can
read into memory there. Such higher-level input files can be read outside of
`ui.c'. This will also be easier later when you add row-concatenation, and
also later when you allow many tables. Also, you have added the sanity check
in `ui_preparations', but the check should be in `concat_along_row' (when the
input table's final set of rows is found).


Reply to this item at:


  Message sent via Savannah

reply via email to

[Prev in Thread] Current Thread [Next in Thread]