GNU Astronomy Utilities - Tasks: task #14409, Polygon vertices from a DS9 region...
You are not allowed to post comments on this tracker with your current authentication level.
task #14409: Polygon vertices from a DS9 region file
Submitter: | Mohammad Akhlaghi <makhlaghi> | ||
Submitted: | Thu 16 Mar 2017 10:50:31 PM UTC | ||
Should Start On: | Thu 16 Mar 2017 03:00:00 PM UTC | Should be Finished on: | Thu 16 Mar 2017 03:00:00 PM UTC |
Category: | Libraries | Priority: | 5 - Normal |
Item Group: | Enhancement | Status: | Postponed |
Privacy: | Public | Percent Complete: | 100% |
Assigned to: | ndanzanello | Open/Closed: | Closed |
Effort: | 0.00 |
( Jump to the original submission )
Sat 22 May 2021 03:47:31 AM UTC, comment #8: |
Mohammad Akhlaghi <makhlaghi>![]() |
Wed 28 Apr 2021 10:25:28 PM UTC, comment #7: Hello Mohammad,
Thank you.
About my copyright assignment, I have already sent it to the GNU Copyright clerk.
I have pushed the changes in which I made a small check in 'gal_options_parse_colon_sep_csv' and now we can specify the filename directly to '--polygon'. I also have updated the documentation. Let me know what you think.
|
Natáli Anzanello <ndanzanello>![]() ![]() |
Mon 19 Apr 2021 01:47:29 AM UTC, comment #6: 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. 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 (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':
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:
|
Mohammad Akhlaghi <makhlaghi>![]() |
Sun 18 Apr 2021 12:38:01 PM UTC, comment #5: Hi Mohammad!
Sorry about that. I didn't notice that the AutoFillMode was not set. (I'm getting to know Emacs)
Anyway, I've re-formated the commit messages :)
Thanks.
|
Natáli Anzanello <ndanzanello>![]() ![]() |
Sun 18 Apr 2021 02:26:31 AM UTC, comment #4: Thanks Natáli!
I pulled the commits and had a look, the changes look good and but when I saw the commit messages, I couldn't continue! In terms of content they are good, the problem is in the length of each line.
Please run 'git log' and compare your commit messages to the previous commit messages ;-). You will see that no commit message line is longer than 75 characters. This is intentional because we want the output of 'git log' to be readable in a standard 80-character terminal window.
Can you please re-format the commit messages to follow the same convention?
Recall that these development commits will later go into Gnuastro's history, so we need to have them in a nice and common format (the history of the project is also very important). |
Mohammad Akhlaghi <makhlaghi>![]() |
Sat 17 Apr 2021 08:22:42 PM UTC, comment #3: Hello!
I've made two commits to the 'crop-polygon-ds9-file' branch.
The first is an implementation of the '--polygonname' option to Table.
The second is the documentation of the new option and the new ds9 library.
Let me know if there are any improvements that can be done :) |
Natáli Anzanello <ndanzanello>![]() ![]() |
Sat 10 Apr 2021 10:04:04 PM UTC, comment #2: This will be a VERY USEFUL feature in Crop, thanks a lot Natáli!
I also ran it and it worked perfectly! Nice job ;-).
However, the function could benefit from some polishing and as I went through the code, I also noticed some important points that are now committed over your commit in Commit 6d5db432a79 (commit message shown below in P.S.)
Almost all of the points are natural for a first time contribution to a project, so don't worry about the number of points ;-). So I tried to elaborate/explain everything a lot for you so hopefully they are not repeated in the future.
In terms of Crop, the work is almost done now. But just to get your hands a little more dirty into the code, let me make an easy proposal: the Table program also has a '--polygon' option that reads polygon vertices and extracts rows that are within/outside the polygon. So this function can also be used there. To generalize the process, in Commit 5d47ed443ed27, I moved this function into a new Gnuastro library file called 'lib/ds9.c' and corrected its calling in 'bin/crop/ui.c'. I also generalized this task to the "Libraries".
So the remaining steps to get this task done are the following (very easy):
If you are applying for GSoC 2021, you can do these after the application in case you don't have time ;-).
P.S. Commit 6d5db432a79 message:
P.Ss: Message of Commit 5d47ed443ed27:
|
Mohammad Akhlaghi <makhlaghi>![]() |
Sat 10 Apr 2021 04:03:22 PM UTC, comment #1: Hello!
I have created a new option (named polygonname, -t for short) in which we are able to use the ds9 filename as an input. I have edited the ui.c (along with ui.h, args.h and main.h) to parse the coordinates and put it on the polygon parameter of the main structure of the program (p->polygon).
I ran some tests and it worked. I have also made a fork and a branch called crop-polygon-ds9-file: https://gitlab.com/ndanzanello/gnuastro-ndanzanello
Looking forward to know what you think about it and what can be improved :)
Thanks! |
Natáli Anzanello <ndanzanello>![]() ![]() |
Thu 16 Mar 2017 10:50:31 PM UTC, original submission:
Allow users to use a file for polygon vertices instead of having to type all the coordinates in by hand. Typing coordinates (especially for larger number of vertices) can be prone to errors and is frustrating.
However, in GUI tools like SAO DS9, we can easily define a polygon region on the image and save the vertices into a file with specific formats. So when the input is a file name (not a string of numbers), Crop can parse the output of DS9 directly and users don't have to deal with the actual numbers. This will greatly speed up the process and reduces chances of error.
Until then, a script like this can be used to read the DS9 region file (saved in the `ds9' format in `fk5', see P.S. below) and prepare it for Crop (new name for ImageCrop) as a shell variable):
This task was suggested by Guillaume Mahler.
---
To define a region file in SAO DS9, you can take the following steps: Open the image with DS9 and activate its "Region" mode with "Edit --> click --> Region". Then define the region to be a polygon with "Region --> Shape --> Polygon". Click on the approximate center of the region you want and a small square will appear. By clicking on the vertices of the square you can shrink or expand it, clicking and dragging anywhere on the edges will enable you to define a new vertice.
After the region has been nicely defined, save it as a file with "Region --> Save" Regions. You can then select the name and address of the output file, keep the format as "REG" and press "OK". In the next window, keep format as "ds9" and "Coordinate System" as "fk5". A plain text file (let's call it `ds9.reg') is now created that you can feed into the script above. |
Mohammad Akhlaghi <makhlaghi>![]() |
No files currently attached
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
I was finally able to review the final commits and merge it in to the 'master' branch. So the task is now being marked as closed :-).