[task #15128] TPV and SIP keywords when WCS distortions are necessary

[Mohammad Akhlaghi]

Follow-up Comment #34, task #15128 (project gnuastro):

Great! Thanks Sachin.

I just went through the changes and made some corrections, you can see them in
commit 68f0c071d7e of my fork
<https://gitlab.com/makhlaghi/gnuastro-dev/-/commit/68f0c071d7ed45cd69429c356d0f54acc2ec2890>
on a branch with the same name. You can easily fetch this commit over your
branch and build over it. Just for reference, I am copying the commit message
here (because that commit is on a temporary branch that will be removed in the
future after rebasing).

Can you please correct all the remaining points and let me know then we can go
into more details ;-).


The corrections can be summarized as follows:

 - No TAB should be used anywhere except Makefiles (there was one in
   'THANKS').

 - Alignment is generally very important (the names in the copyright
   statement of 'wcsdistortion.h' were not under each other).

 - The file summary at the top of 'wcsdistortion.h' was wrong.

 - In a '.h' file, only things that are related to OUTSIDE the
   corresponding '.c' file should be included. So the 'max', 'min',
   'gal_wcsdistortion_calc_tpveq' and 'gal_wcsdistortion_calcsip' shouldn't
   be in 'wcsdistortion.h'. Such header-like features that are only for the
   '.c' file should be put at the start of the '.c' file.

 - In 'gal_wcs_distortion_identify', we now simply return the corresponding
   macro, instead of saving it to an integer value and returning that, it
   is cleaner like this.

 - There is a problem in 'gal_wcs_distortion_identify' (as mentioned in the
   comments, after 'PROBLEM:').

 - All functions in 'wcsdistortions' (except the two that go into its
   header) must be static (some weren't), AND, they must not start with
   'gal_'. The 'gal_' prefix is only for functions that are go into the
   headers (used in other files).

 - In 'wcsdistortion_intermidate_tpveq', the '+' is put at the end of the
   each line, followed by a '\'. In C when the line isn't a macro we don't
   need a '\'. When you want to break a statement, its more readable if you
   put it in an outer parenthesis and put the statements under that.

 - Following the previous point, according to the GNU Coding Standards,
   when you break an expression, its better to put the operator at the
   start of the next line, rather than the end of the previous line. This
   is much more readable and highlights the operator ('+') in this case.

 - I done the above two points for 'k[0][2]' and 'l[0][2]', but please
   correct them for the rest also ;-).

 - No line should be longer than 75 characters, unless there is absolutely
   no other way! So the 'sprintf' statements in
   'wcsdistortion_add_sipkeywords' were corrected. Please correct the
   similar statements for the other function also.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/task/?15128>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/