[task #14409] Polygon vertices from a DS9 region file

[Mohammad Akhlaghi]

Follow-up Comment #6, task #14409 (project gnuastro):

Thanks a lot Natáli, 

I really liked your guide on making a polygon region file in DS9, very nice
and complete! Generally, your work on the documentation was very impressive
:-).

Before getting to the changes I made, let me also mention this: looking at the
commit messages, I noticed that in the second one, the title line ended with a
period (.)! This is also against our commit conventions (as with many other
projects: its just like a title of a section in a book/paper: titles don't end
with a period). So I '--amended' your commit and '--force' pushed it to Gitlab
in Commit 2c3731114
<https://gitlab.com/makhlaghi/gnuastro-dev/-/commit/2c3731114>. So before
continuing the development of this branch, pull this new commit and work on
top of this ;-).

You had also mentioned that this message is a re-write of a previous one.
Don't forget that the previous one is no longer in the history and nothing but
the format has changed, so this will only confuse the future readers of the
history ;-). So I removed it in the edit.

After building and testing over the commits, I made some improvements in
Commit 36e8b53 <https://gitlab.com/makhlaghi/gnuastro-dev/-/commit/36e8b53>
(commit message shown in P.S.). Please study the changes carefully to help
smooth your future commits for merging :-).

Generally, I feel this is fine to be merged into 'master'. The only remaining
thing is your copyright statement. Please stay in touch with the GNU Copyright
clerk (and remind them if necessary), so that can be done as soon as
possible.

One possible/optional improvement that occurs to me is this (to help use the
time until the copyright is assigned): since file names are very different
from numbers, we can remove the new '--polygonfile' overall and let the user
specify the file directly with '--polygon':


--polygon=1.234,5.678:1.234,5.678:1.234,5.678
--polygon=filename


When the user gives something like '--polygon=filename', instead of parsing it
as a series of numbers with 'gal_options_parse_colon_sep_csv_raw', we can
immediately give the string to 'gal_ds9_reg_read_polygon'. 

To do this, a small check has to be added in 'gal_options_parse_colon_sep_csv'
(in 'lib/options.c'). For example if a comma (,) and colon (:) exist in the
string, we can interpret the value to '--polygon' as a series of numbers.
Otherwise, we can interpret the value to '--polygon' as a filename.

The coordinate mode can be written into the 'status' component of the output
'gal_data_t' afterwards (it was precisely defined for such situations).

This would also be much more user-friendly ;-).

P.S. Message of Commit 36e8b53
<https://gitlab.com/makhlaghi/gnuastro-dev/-/commit/36e8b53>:


    Crop and Table: --polygonfile new name for --polygonname
    
    Until now, the '--polygonname' option was used to specify the SAO DS9
    region file that contains the polygon to use. However, this could be
    confused with the actual name of some polygons that have a name: for
    example "triangle" or "octagon"!
    
    With this commit, the option has been renamed to '--polygonfile' in both
    Crop and Table. Also, I noticed that the short '-t' option was only used
    for Crop and not Table. Generally, when there is a similar option between
    two programs, they should behave similarly to avoid confusion for the
    users. So the short format of this option was removed in Crop is well.
    
    In the process, some other changes have been made which are listed below:
    
     - The NEWS file has been updated with all the new features added.
    
     - In Table, the coordinate system of the vertices is irrelevant: the
user
       specifies the columns to check with and there is no way we can know if
       they are image or WCS. So the check on having a reasonable coordinate
       has been removed. But generally, Natáli, when you want to break a
       conditional into two lines, bring the conditional operator to the
start
       of the next/second line, not the end of the first line. This helps a
lot
       in readability (the eye is much less prone to miss something at the
       start of a line, rather than at the end of it), like below:
    
          if( ds9regmode!=GAL_DS9_COORD_MODE_IMG
              && ds9regmode!=GAL_DS9_COORD_MODE_WCS )
    
     - In the book, I generally noticed that sometimes "SAO ds9" was used and
       some times "SAO DS9" (long before this branch). So to unify things,
they
       are all now "SAO DS9" (following its own webpage).
    
     - In the book, some index items were added (with '@cindex').
    
     - In the book, the expected format of the region file has been taken to
       the 'gal_ds9_reg_read_polygon' function. In the description of
       '--polygonfile', it only points the interested reader to there. This
       helps most users of Crop or Table (a high-level user will generally
not
       be interested in low-level things and can even find them annoying and
       distracting).
    
     - In the book, the polygon line of the example region file was too long
       for the printed format (created with 'make pdf'). So I shortened the
       example vertices.
    
     - In the book, at the start of the library section on SAO DS9, there was
       no description of what SAO DS9 actually is. I know its clear for most
       astronomers, but the library is sometimes inspected by
non-astronomers,
       so its always good to start each section with the most basic
description
       and explain a little more on what the whole purpose is for someone
with
       no background.
    
     - In the book, the three macros were brought into one '@deffn' (second
and
       third with a '@deffnx') Since they are related, this helps in
       readability and avoids the need to repeat things in the description.



    _______________________________________________________

Reply to this item at:

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

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