taskGNU Astronomy Utilities - Tasks: task #15317, Concatenate two or more tables by...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #15317: Concatenate two or more tables by row

Submitted by:  Raul Infante-Sainz <infantesainz>
Submitted on:  Wed 03 Jul 2019 07:52:04 PM UTC  
 
Should Start On:  Wed 03 Jul 2019 12:00:00 AM UTC Should be Finished on:  Wed 03 Jul 2019 12:00:00 AM UTC
Category:  Table Priority:  5 - Normal
Item Group:  New feature Status:  None
Privacy:  Public Percent Complete:  0%
Assigned to:  None Open/Closed:  Open
Effort:  0.00

( Jump to the original submission)

Sun 22 Mar 2020 03:11:45 PM UTC, comment #39: 

Thanks! It has been merged and pushed to the main repository.

I also followed it up by a commit, polishing it a little and also enabling multiple calls to the `--catcolumn' option to add multiple tables. Please have a look at the commit message and changes to hopefully have smoother merges in the future.

I changed the task name to only focus on the rows now. If you get the chance to also implement that feature, we can close this task when its merged ;-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Sun 22 Mar 2020 02:40:18 AM UTC, comment #38: 
madhav_bansal <madhav_bansal>
Project Member
Sat 21 Mar 2020 11:49:46 PM UTC, comment #37: 

Madhav, given that your copyright forms are complete, I am ready to merge, but I noticed that your commit was in parallel to another commit that I just merged into master.

To have a cleaner history, can you please pull the master branch to the most recent commit, the rebase your commit over it? Let me know when its done and I'll do the merge ;-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Fri 20 Mar 2020 05:43:59 PM UTC, comment #36: 

Yes, that is correct, in many scenarios a file has multiple HDUs/extensions and the user may even want to concatenate two tables in one file for example.

OK! Everything is ready :-D. I'll merge it into the master branch as soon as the copyright things are done. Just post a comment here when its done. If you think its taking long, you can send them another email and remind them that you want your work merged before the GSoC deadline ;-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Fri 20 Mar 2020 03:33:02 PM UTC, comment #35: 

done.
https://github.com/madhav-bansal/gnuastro/commits/table
We are including this option for giving user the choice to choose different HDU for -catcolumn file right?. So that INPUTFILE and --catcolum can have different HDU's?

madhav_bansal <madhav_bansal>
Project Member
Fri 20 Mar 2020 02:49:31 PM UTC, comment #34: 

The options of `lib/gnuastro-internal/commonopts.h' are common to all programs, so you shouldn't add it there! You should have added it to the `args.h' of the Table program ;-). Can you please correct this?

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Fri 20 Mar 2020 06:35:20 AM UTC, comment #33: 

I have inserted the option in (lib/gnuastro-internal/commonopts.h)and also check the documentation for --catcolhdu is it correct?
https://github.com/madhav-bansal/gnuastro/commits/table

madhav_bansal <madhav_bansal>
Project Member
Wed 18 Mar 2020 10:22:21 PM UTC, comment #32: 

Thanks, I gave it a try and looked at the code, it is almost ready to be merged. But just now something occurred to me: when calling `gal_table_read', you are giving it the correct file name, but not the correct HDU.

The `cp->hdu' is actually the HDU of the input file, not the file to be concatenated. You should define a new option, maybe called `--catcolhdu' (defined similar to the `--hdu' option (defined in `lib/gnuastro-internal/commonopts.h'). and feed its string instead of `cp->hdu'. This will enable users to have their desired concatenation table in any extension they want. Also don't forget to document this new option also.

Finally, while you are making corrections to this code, please break down the error message such that each line doesn't exceed 75 characters (the rest of the function is good, don't let the lines of the error message get any longer per line).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Wed 18 Mar 2020 04:15:06 PM UTC, comment #31: 
madhav_bansal <madhav_bansal>
Project Member
Wed 18 Mar 2020 03:09:52 PM UTC, comment #30: 

Thanks Madhav, the explanation is more clear now. You can commit and rebase it now (so all your work comes into one commit after the most recent master branch commit) then let me know. I'll then give it one last try and as soon as your copyright forms are complete, I'll merge it into the master branch :-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Wed 18 Mar 2020 11:36:07 AM UTC, comment #29: 

I think this explains the option correctly
"File whose columns to add in the main INPUTFILE(keeping number of rows fixed).
If any row selection is applied then first row selection is applied on main INPUTFILE and then concatenation is done."
please see if it correctly explains?

I have mailed again, for the copyright confirmation today.

madhav_bansal <madhav_bansal>
Project Member
Tue 17 Mar 2020 01:29:51 AM UTC, comment #28: 

I actually checked out to your branch this time and ran it. Things look good now :-). Only a small problem: I see that the added documentation again has lines ending with white-space.

Generally, its good to make this a rule of thumb: before pushing your commits, run `git log -p -1' and look at what you have committed. Following the discussion before professional editors have modes to remove such things (another is an indentation with TAB, which is very bad except for Makefiles) every time the file is saved. For example I have activated it in Emacs for myself, so I never have to worry about it ;-).

Besides the ending with whitespace, the description of the option can be improved: 1) its English, 2) note in other parts of `gnuastro.texi' that we have the convention of one-sentence-per-line. This greatly helps in managing changes in this long documentation with Git!

Once you apply these final changes I am ready to merge, but I just need the Copyright confirmation first. Can you please send them another email and confirm if they are pursuing your case?

About the many things you can add to Table, there is a list already a list of table-specific features in the tasks (especially if you select the "Category" to be table).

But these are just the top of the ice burg! The main tool people currently use for Table processing in astronomy is TOPCAT. Besides its visualization features, it also has many useful processing features that we can also add to table. The problem with TOPCAT is that its written in Java, and thus not usable by anyone else! In Gnuastro useful features can be added to the library and used in many other contexts, besides the simple and easy to use Table program ofcourse.

Another interesting program that heavily uses tables is Match, with its own set of features that may need adding. Gnuastro's MakeCatalog is also the interface between images and tables, with MANY useful operators/measurements to add.

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Tue 17 Mar 2020 01:09:38 AM UTC, comment #27: 

By the way, you can check the documenation with this command:

make pdf

The generated PDF file (if TeX doesn't crash!) is saved in `doc/gnuastro.pdf'.

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Tue 17 Mar 2020 12:56:18 AM UTC, comment #26: 

I'll look at it in a few minutes, but most editors have a feature to automatically adjust plain-text widths, for example if you use Emacs, you can simply press Alt-Q and the paragraph text will align. Vim also has a similar feature, I don't know about other editors. But generally try to use a professional text editor and master it to greatly save your time and energy in coding ;-).

I recommend GNU Emacs, but won't get into the Editor war: Vim is also good! Its just important to master one and benefit from all its powerful features.

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Tue 17 Mar 2020 12:51:15 AM UTC, comment #25: 

sorry, for making the same mistake again and again
Now I have done all the things correctly.I actually was having trouble with commit message width.
https://github.com/madhav-bansal/gnuastro/commits/table

madhav_bansal <madhav_bansal>
Project Member
Tue 17 Mar 2020 12:40:23 AM UTC, comment #24: 

I have updated the documentation also, but not sure if did it right. Is there any way to check how it looks as output.
https://github.com/madhav-bansal/gnuastro/commits/table
Also, I wanted to ask is it fixed or something that you have to solve these numbers of fixed tasks in the GSoC proposal.
Also If I want to improve table program as a gsoc what possible changes I can add in a table program, as I think I have a good grip over it.

madhav_bansal <madhav_bansal>
Project Member
Tue 17 Mar 2020 12:04:37 AM UTC, comment #23: 

Good, but still not enough! Please pay attention to all the other commits/codes/comments in the code. You want your code to fully blend into the program's source.

  1. The commit message body description is now unreasonably short! After making the commit, please run `git log' and press SPACE (to see other messages). Then re-edit it (with `git commit --amend'), so it and repeat this until it looks like the rest.
  2. In `tableparams', please look at the other comments: do you notice that the comment ending (`*/') in all the rest of the structure elements are aligned? Please do it for this line also.
  3. No other name in the lists I mentioned before are FULL CAPS! Can you please correct your name to be in the standard notation (only first character to be capital).
  4. Finally: the documentation has not been added.

It is a little frustrating for me to see a previously raised point is being repeated multiple times. Please pay more attention  if you are serious about this ;-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Mon 16 Mar 2020 11:31:54 PM UTC, comment #22: 

I have made commit message little sorter and corrected the other two points also.
https://github.com/madhav-bansal/gnuastro/commits/table

madhav_bansal <madhav_bansal>
Project Member
Mon 16 Mar 2020 11:17:45 PM UTC, comment #21: 

Another (possibly) important point: I see that in the commit info, your name is "madhav-bansal". We use this name to generate the `AUTHORS' file, and also print it in the second page of the final PDF. If you are happy with "madhav-bansal" that is fine, but I just thought of bringing it up in case you want your standard name string to be put in those places ;-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Mon 16 Mar 2020 11:14:05 PM UTC, comment #20: 

A few other small touches:

  1. The commit message is much better now. Good explanation ;-). I just feel the third line of the message body (ending in `"--catcolumn"') is too long! The basic idea is this: on an 80-character terminal, the output of `git log' should be easily readable. 80-characters is a traditional default of many virtual terminals, people may make it larger, but few will probably make it smaller. So its good to keep your lines less than 75 characters (given the indentation that Git adds to the log message).
  2. The option description in `program_options' is still not corrected (point 4 in Comment #11).
  3. Please keep the description of the new option in `tableparams' (`main.h') within the same limit of the description of other options.
Mohammad Akhlaghi <makhlaghi>
Project Administrator
Mon 16 Mar 2020 11:01:40 PM UTC, comment #19: 

well done all the things in the table_catcolumn function itself. please give feedback.https://github.com/madhav-bansal/gnuastro/commits/table

madhav_bansal <madhav_bansal>
Project Member
Mon 16 Mar 2020 10:28:45 PM UTC, comment #18: 

Of course, if a task is VERY INTERESTING for you and you feel you an do it better, you can also write your GSoC proposed project based on a task that someone else has already expressed interest on. But be careful! The competition (and thus possibility of failure) will increase ;-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Mon 16 Mar 2020 10:07:21 PM UTC, comment #17: 

Great! I look forward to the corrections ;-).

Its good to also add the documentation in the same commit so it can be complete.

For the GSoC proposal, once your first commit is merged into the main history of Gnuastro, I think you can make a good case that you know the source code and the workflow. But you also need to choose a useful/important project to propose to work on in Gnuastro. There are some that are suggested in the GSoC suggestion page, and of course the general task list is very complete. The Table program itself can be greatly improved and has a lot of potential, but there are many other image processing tasks.

I guess its really up to you and what you are interested in, so have a general look and if something is not taken by others, you can post a comment that you will be working on it and write your proposal.

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Mon 16 Mar 2020 09:56:51 PM UTC, comment #16: 

Thank you.
ok, I will try to correct these things.
Do need to write the documentation in the same  one commit, or can make just another commit about documentation.
Also please guide about writing proposal for gnuastro for gsoc.

madhav_bansal <madhav_bansal>
Project Member
Mon 16 Mar 2020 09:49:47 PM UTC, comment #15: 

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:

  1. 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).
  2. 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.
  3. In `concat_along_row', you can use the `gal_list_data_last' function (in list.h) to avoid the while loop and help in readability.
  4. 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 ;-).
  5. 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?
  6. 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).
Mohammad Akhlaghi <makhlaghi>
Project Administrator
Mon 16 Mar 2020 09:20:56 PM UTC, comment #14: 

Now I have elaborated on the comments and the commit message.
Also, check the number of rows. But not sure that if "rows in output" option thing is solved. please let me know if it is correct of if something is wrong.
https://github.com/madhav-bansal/gnuastro/commits/table

madhav_bansal <madhav_bansal>
Project Member
Mon 16 Mar 2020 07:42:23 PM UTC, comment #13: 

It looks better, thanks! But some of the points haven't been fully/correctly applied:

  1. The commit message should be more elaborate! This also applies to comments: please think about others (including your future-self) is well! People read these commit messages and don't necessarily have the time review the code to see exactly what you mean by "Added an option to concatenate columns of two tables". Describe it at least in three sentences and the conditions that it can be useful. If you are not yet convinced of the importance of commit message, or comments, please see this nice word of wisdom.
  2. On the output of `git log', I still see regions marked in red! Showing lines that end with white-space characters.
  3. The option name is fixed, it is still identified with `UI_KEY_SECONDFILE'! This will later cause so many confusions! The name of the identifier and option must be the same.
  4. The option description hasn't yet changed.

Some other points:

  1. I don't see any sanity checks: what if the number of rows in the new table doesn't correspond to the number of rows in the original input? Table should abort with a descriptive error message, saying the problem (while also printing the numbers).
  2. The user may first select a few of the original table's rows (to fit another table), then concatenate it. So it is best to read the table to be concatenated after all the row-selection procedures on the main table has been done (for example all the options under "Rows in output" when you run it with `--help'). For example take this scenario: `base.fits' has 100 rows, while `other.fits' has 10 rows. I run one of the operations mentioned above on `base.fits' (for example `--range'), then I want to merge it with `other.fits'. To allow this, its easier if the function to do this job is moved to the end and the other table is read and sanity-checked in the same function.
  3. Also, don't forget to add documentation for this new option. You can do that in `doc/gnuastro.texi'. Its in GNU Texinfo format which is essentially TeX but with an `@' instead of a backslash. You can use the other option descriptions as a reference without going into the details of Texinfo. You don't need any complex Texinfo feature for this ;-).
Mohammad Akhlaghi <makhlaghi>
Project Administrator
Mon 16 Mar 2020 07:09:37 PM UTC, comment #12: 

I have tried to correct the points directed by you .please check https://github.com/madhav-bansal/gnuastro/tree/table

madhav_bansal <madhav_bansal>
Project Member
Sat 14 Mar 2020 06:18:22 PM UTC, comment #11: 

No problem ;-). I am just looking at it, here are some points I am wrote as I was inspecting your commit. They are almost all cosmetic, so it won't take much time/effort to implement them. Nevertheless they are important so please implement them and keep them in mind for future commits.

  1. The commit message is a single long line. Please have a look at our Commit guidelines (in the Developing chapter). Even when you want to rebase later (and write a clean commit message for many commits merged into one), it is good that you follow these guidelines, so it becomes a habit.
  2. When I do a `git log' on you commit, I see that Git has marked the lines with an empty space in the end. This is generally is a very bad practice and higly discouraged (that is why even Git highlights them). Please run `git log' after doing your commit (and before pushing) to see and remove such cases in future commits. Generally most editors have modes where they automatically remove them. You can consider activating those modes.
  3. You have called the new option `secondfile'. But this is too general. Why not follow the option names I suggested in a previous comment (`--catcolumns' for files that concatenate columns and `--catrows' for files that concatenate rows)? Or something like them? Such names clearly identify the job.
  4. The option description starts with a space, doesn't start with a capital letter and end with a point. Please have a look at the other options and follow the same standards.
  5. You may have noticed that in Gnuastro we don't use the C++ style comments (`//'). Although it is in C since C99, we have followed the basic convention of using the standard C comments (`/* */') throughout the code. Please correct them to this style.
  6. In `ui_preparations', I see that you haven't followed the standard indentation (where the statement after an `if' should start after two space characters). Please have a look at other `if' statements throughout the file and follow this convention.
  7. In `ui.h', you added the new `f' character for the option's short name, but didn't remove it as a free character from the list right above it. If you plan on keeping it after correcting the option names, please also remove it from that list for future options.
  8. The file `doc/version.texi.tmp22936' has been commited! This seems to be a temporary file for your system/build. It shouldn't be committed into the whole history.
  9. Test changes in `tests/during-dev.sh' shouldn't be committed in the end. For now (before you rebase your code into one commit) its fine, but in the end, changes in this file shouldn't be in the commit.
Mohammad Akhlaghi <makhlaghi>
Project Administrator
Sat 14 Mar 2020 05:45:25 PM UTC, comment #10: 
madhav_bansal <madhav_bansal>
Project Member
Sat 14 Mar 2020 05:42:18 PM UTC, comment #9: 

I am happy to hear that you have a first implementation. But I don't see any link to your forked Gnuastro (as a Git project) to see what you have done. Can you please post the Git repository link?

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Sat 14 Mar 2020 01:05:41 PM UTC, comment #8: 

i added the concatenate column option ....please give feedback on it.It is for one table only and i am thinking to use "struct list_select " to store tables for merging many tables.please give feedback about it

madhav_bansal <madhav_bansal>
Project Member
Fri 13 Mar 2020 12:10:50 PM UTC, comment #7: 

I encourage you to read the Developing chapter of the book. In particular the Program source and Mandatory source code files section and subsection. There, you'll see an explanation of all the basic program source files.

If you also have a look at the Gnuastro library section, you will see how you can find the definition of the macros and library functions. For example, you will see that `GAL_TYPE_STRLL' is defined in the Library data types section.

Adding new options is easy once you get use to it, but I haven't had too much time to document it yet unfortunately. In principle if you follow the functions from `main.c' you will see how the program enters `ui.c', and how it starts parsing the options.

But here is a summary of steps to add new options (I'll put this into the book later):

  1. Add an entry into `args.h', just like the `column' option that you cited. The easiest case is to copy a similar option and re-set some of its parts. For two options you want to add, the `column' option is a good reference to copy.
  2. Go into `ui.h' and add a `UI_KEY_*' element with the same name as your new option. This is the integer "key" that is used to identify your new option. Then use that as the second element.
  3. Update the option description (4th element) to describe what the option does. This is printed with the `--help' option.
  4. If you option belongs to a different "group" of options (see `--help' for the grouping), change the group key.
  5. Go to `main.h' and define an element in the main program structure to host the values (in the case of the `--column' option, this is the `p->columns' element).
  6. The "type" of the value is specified by the 8th element. For example if you want you option to only read a single, 32-bit floating point, you would put `GAL_TYPE_FLOAT32'.
  7. If you have chosen a sufficiently similar option as a reference to copy from, you won't need to change the final set of elements. But generally, you can read about each element of this structure under in the comments of the `argp_option' structure in `bootstrapped/lib/argp.h'.
  8. If the program then compiles, you can simply pass the new option and check/use its values in the program (after `ui.c') by looking into the element you defined in `main.h' (`p->columns' in the case of the `--column' option). All elements of the program's main structure get initialized to a 0 (for numbers) or NULL (for pointers). So if your new function is a pointer (like this case), one way to see if the user called your new option is to check if its NULL or not.

I hope I haven't missed anything ;-).

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Fri 13 Mar 2020 10:40:06 AM UTC, comment #6: 

I actually I am having a problem on understanding the args.h
like in this option of "column" in args.h of table module.
what the things are written at last "GAL_TYPE_STRLL" mean?

{
      "column",
      UI_KEY_COLUMN,
      "STR",
      0,
      "Column number (counting from 1) or search string.",
      GAL_OPTIONS_GROUP_INPUT,
      &p->columns,
      GAL_TYPE_STRLL,
      GAL_OPTIONS_RANGE_ANY,
      GAL_OPTIONS_NOT_MANDATORY,
      GAL_OPTIONS_NOT_SET
    },

madhav_bansal <madhav_bansal>
Project Member
Fri 13 Mar 2020 09:51:03 AM UTC, comment #5: 

I am having a problem on understanding how the command line arguments are parsed in the gnuastro. I think it is done in ui.c file.
Also please tell what the file args.h is about

madhav_bansal <madhav_bansal>
Project Member
Fri 13 Mar 2020 12:52:24 AM UTC, comment #4: 

Thanks Madhav, This seems like a good implementation for concatenating columns (where the number of rows doesn't change). I am happy to see you have such a good understanding of Gnuastro's core data structure ;-).

When concatenating rows (such that the number of columns doesn't change), you can use the column metadata of the first input.

After the functions are implemented, this operation can be done like the command below. When we want to concatenate the columns of `table-add.fits' with `table-base.fits'.

$ asttable table-base.fits --catcolumns=table-add.fits

Or this way to concatenate rows:

$ asttable table-base.fits --catrows=table-add.fits

Some checks also need to be done on the sizes to make sure that it is possible.

Finally, we can implement these two options so they can be called multiple times (and merge multiple tables). For example

$ asttable table-base.fits --catrows=table-add-1.fits --catrows=table-add-2.fits

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Thu 12 Mar 2020 08:59:52 AM UTC, comment #3: 

Also, suggest what to do with the other meta-data while concatenating the tables along  the row

madhav_bansal <madhav_bansal>
Project Member
Wed 11 Mar 2020 06:37:58 AM UTC, comment #2: 

does it seems right to you for adding columns to existing rows

static void concat_along_row(gal_data_t *first_table,gal_data_t *second_table){

         gal_data_t *tmp=first_table;
        //traversing to the end column of the first table

        while(tmp->next!=NULL){
                tmp=tmp->next;
        }

        //connecting the last column of first table to first column of second table
        tmp->next=second_table;

}

I have just connected the last column of the first table to the first column of second table
please give suggestions?

madhav_bansal <madhav_bansal>
Project Member
Fri 14 Feb 2020 01:53:00 PM UTC, comment #1: 

This is an interesting idea, but we can make it more general: the `tcat' option in STILTS, adds rows to existing columns. Besides that, we can also have an option to add columns to existing rows.

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Wed 03 Jul 2019 07:52:04 PM UTC, original submission:  

It would be worth to have an option in `Table' to be able to concatenate two or more tables. Something similar to what `tcat' of STILTS (http://www.star.bris.ac.uk/~mbt/stilts/sun256/tcat.html) does.

Raul Infante-Sainz <infantesainz>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by madhav_bansal (Posted a comment)
  • -email is unavailable- added by makhlaghi (Posted a comment)
  • -email is unavailable- added by infantesainz (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-03-22 makhlaghi SummaryConcatenate two or more tables => Concatenate two or more tables by row

    Back to the top


    Powered by Savane 3.5