Write regions and catalogs #36

Athanaseus · 2024-11-08T16:25:24Z

Adds --outcatalog and --outregion to write outt catalog text file and save regions file respectively.
Update README and add GitHub pages

NB: The catalogs are also compatible with the Tigger viewer

remove the commented section except for the column headings

# processing fits image: deep2-MFS-image.fits
# mean beam size (arcsec): 6.31 
# original image peak flux (Jy/beam): 0.023107271641492844 
# noise out (µJy/beam): 51.62 
# cutt-off flux  (mJy/beam): 0.52 
# freq0 (Hz): 1049833007.8125 
# number of sources detected: 100 
#
#format: name ra_d dec_d i i_err emaj_s emin_s pa_d
src0 60.64975237181635 -79.86348735299585 0.00369 0.0001 11.66 0.0 120.96
src1 60.67140679629887 -80.36126947232378 7e-05 0.0001 0.0 0.0 0.0
src2 60.877334392876136 -79.76691180042988 0.00113 0.0001 8.25 0.0 104.04

…reizorro into write_regions_catalogs

o-smirnov · 2024-12-13T10:31:22Z

breizorro/breizorro.py

+    parser.add_argument('-ncpu', '--ncpu', dest='ncpu', default=None, type=int,
+                        help='Number of processors to use for cataloging.')
+    parser.add_argument('-beam', '--beam-size', dest='beam', default=None,
+                        help='Average beam size in arcesc incase beam info is missing in image header.')


I think this breaks with our current CLI convention of only single-letter options for "-". I would use "-j" for ncpu (it's a popular Unix alias) and "-B" for beam.

o-smirnov · 2024-12-13T10:32:08Z

breizorro/breizorro.py

+            from breizorro.catalog import multiprocess_contours
+        except ModuleNotFoundError:
+            LOGGER.error("Running breizorro source detector requires optional dependencies, please re-install with: pip install breizorro[catalog]")
+            raise('Missing cataloguing dependencies')


Maybe raise and log the same message?

o-smirnov · 2024-12-13T10:32:52Z

breizorro/breizorro.py

+            # Suppress FittingWarnings from Astropy
+            # WARNING: The fit may be unsuccessful; check fit_info['message'] for more information. [astropy.modeling.fitting]
+            warnings.resetwarnings()
+            warnings.filterwarnings('ignore', category=UserWarning, append=True)


I think there's a context manager form (with something) to temporarily change warnings settings.

o-smirnov · 2024-12-13T10:34:20Z

@Athanaseus I wonder -- the CLI's getting quite complex, is this not a good time to move it to a YAML schema and switch to clickify_parameters instead?

o-smirnov · 2024-12-14T09:15:53Z

breizorro/breizorro.py

-        parser.error("Either --restored-image or --mask-image must be specified")
+
+    # Example usage of the parameters
+    LOGGER.info(f"Restored Image: {restored_image}")


In the production version, printing all this is probably not necessary.

o-smirnov · 2024-12-14T09:18:45Z

breizorro/config/breizorro.yaml

@@ -0,0 +1,120 @@
+opts:


opts should not be in the cab definition. Cab def only. Also, suggest placing this at top level, so it can be included as _include: (breizorro)breizorro.yml.

o-smirnov · 2024-12-14T09:19:10Z

breizorro/config/breizorro.yaml

+      restored_image:
+        info: "Restored image file from which to build the mask"
+        dtype: File
+        required: false


required: false is the default for all inputs, so can probably omit it.

o-smirnov · 2024-12-14T09:20:08Z

breizorro/config/breizorro.yaml

+        dtype: bool
+        required: false
+        default: false
+      dilate:


Suggest adding metavar: R to this one to maintain consistency with old CLI (anywhere where we defined a metavar).

o-smirnov · 2024-12-14T09:22:09Z

breizorro/config/breizorro.yaml

+        default: false
+      merge:
+        info: "Merge one or more masks or region files"
+        dtype: list[File]


I think this should be List[File], also elsewhere. Also, for multiple options, do we want --merge FILE1 FILE2, --merge FILE1,FILE2 or --merge FILE1 --merge FILE2? Set policies: repeat accordingly, to list, [] or repeat.

Will the following work?

merge: info: "Merge one or more masks or region files" dtype: List[File] policies: repeat: ' '

When repeat is set to list it passes the argument as a string instead of list of files.
Also, this results in no file validation.

Actually I lie, --merge FILE1 FILE2 is not available with click. So you have to decide on --merge 'FILE1 FILE2' (cumbersome because it needs shell quotes), --merge FILE1,FILE2, or --merge FILE1 --merge FILE2.

When repeat is set to list it passes the argument as a string instead of list of files.
Also, this results in no file validation.

File validation is not done via clickify anyway. But it's strange that it passes as a string. Could you please test again? Alternatively, try the clickify-cleanup branch I just pushed to stimela. I've added a test there and it seems to work as expected:

(stimela) oms@baker:~/projects/stimela2/tests/scabha_tests$ ./test_clickify.py 1 2 a b --files1 'x y z' --files2 x --files2 y --files3 a,b --output x 1 2 None 2.0 None remainder: ('a', 'b') files1: ['x', 'y', 'z'] files2: ('x', 'y') files3: ['a', 'b'] output: x (stimela) oms@baker:~/projects/stimela2/tests/scabha_tests$

Alternatively, try the clickify-cleanup branch I just pushed to stimela. I've added a test there and it seems to work as expected:

Now I get the following when I set repeat policy to list: scabha.exceptions.SchemaError: click parameter 'merge': repeat=list policy is only supported for positional=true parameters

I'd go with --merge File1,File2.

merge: info: "Merge one or more masks or region files" dtype: List[File] metavar: "MASK(s)|REG(s)" policies: repeat: '[]'

But for the extract-islands/remove-islands parameters which were previously specified as --remove-islands 19 21 14h16m20.3s,-65d42m07s, can now be configured as:

remove_islands: info: "Remove islands from input mask (list by number or coordinates)" dtype: Optional[Tuple[int, str]] policies: repeat: ' '

which results in --remove-islands '19 21 14h16m20.3s,-65d42m07s'

Now I get the following when I set repeat policy to list: scabha.exceptions.SchemaError: click parameter 'merge': repeat=list policy is only supported for positional=true parameters

Yeah that's as expected. Click does not support variable-length options -- only positional arguments (i.e. the last positional argument can be variable-length, it will swallow "the rest" of the command line.)

But for the extract-islands/remove-islands parameters which were previously specified as --remove-islands 19 21 14h16m20.3s,-65d42m07s, can now be configured as:

remove_islands: info: "Remove islands from input mask (list by number or coordinates)" dtype: Optional[Tuple[int, str]] policies: repeat: ' '

which results in --remove-islands '19 21 14h16m20.3s,-65d42m07s'

I'm confused that this works. Tuple[int, str] should expect two arguments, an int and a str, so if it accepts three, that's a bug in clickify_parameters. I'll investigate. In any case you probably meant List[Union[int, str]], right?

I'm a bit concerned that we have two different ways of specifying multiple arguments (comma-separated and space-separated) in the CLI now, this is potentially confusing. Maybe consider using : as a separator throughout?

I released stimela 2.0.2 which incorporates the CLI fixes.

I'm confused that this works. Tuple[int, str] should expect two arguments, an int and a str, so if it accepts three, that's a bug in clickify_parameters.

I was yet to test this dtype, actually I had it as List[str].

I'll investigate. In any case you probably meant List[Union[int, str]], right?

Exactly.

I'm a bit concerned that we have two different ways of specifying multiple arguments (comma-separated and space-separated) in the CLI now, this is potentially confusing. Maybe consider using : as a separator throughout?

Sounds good to me.

Example:
breizorro -r mypipelinerun-MFS-image.fits--merge mask.fits,adass.reg --number-islands --remove-islands 19,21,14h16m20.3:-65d42m07

breizorro.breizorro - 2024-12-20 00:55:47,393 INFO - Welcome to breizorro breizorro.breizorro - 2024-12-20 00:55:47,444 INFO - Version: 0.1.3 breizorro.breizorro - 2024-12-20 00:55:47,444 INFO - Usage: breizorro --help breizorro.breizorro - 2024-12-20 00:55:47,457 INFO - Generating mask using threshold 6.5 breizorro.breizorro - 2024-12-20 00:55:47,457 INFO - Generating noise map breizorro.breizorro - 2024-12-20 00:55:47,582 INFO - Median noise value is 0.0007369568920694292 WARNING: FITSFixedWarning: 'datfix' made the change 'Set MJD-OBS to 58318.744319 from DATE-OBS'. [astropy.wcs.wcs] breizorro.breizorro - 2024-12-20 00:55:47,615 INFO - Treating mask.fits as a FITS mask breizorro.breizorro - 2024-12-20 00:55:47,622 INFO - Merged into mask breizorro.breizorro - 2024-12-20 00:55:47,894 INFO - Merging in 24 regions from adass.reg breizorro.breizorro - 2024-12-20 00:55:48,506 INFO - (Re)numbering islands breizorro.breizorro - 2024-12-20 00:55:48,524 INFO - Number of islands: 35 breizorro.breizorro - 2024-12-20 00:55:48,524 INFO - Removing islands: ['19', '21', '14h16m20.3:-65d42m07'] breizorro.breizorro - 2024-12-20 00:55:48,550 INFO - coordinates <SkyCoord (ICRS): (ra, dec) in deg (214.08458333, -65.70194444)> correspond to pixel 328, 241 with value 5 breizorro.breizorro - 2024-12-20 00:55:48,568 INFO - Writing mypipelinerun-MFS-image.mask.fits breizorro.breizorro - 2024-12-20 00:55:48,588 INFO - Done

o-smirnov · 2024-12-14T09:22:48Z

breizorro/config/breizorro.yaml

+        info: "Sum-to-peak ratio of flux islands to mask in original image"
+        dtype: float
+        required: false
+      ncpu:


Can add abbreviation: j to have the single-letter -j option also available. Also for other arguments.

In general, we should do this to keep the CLI compatible with older releases.

o-smirnov · 2025-01-13T09:32:33Z

@Athanaseus what's the status of the changes -- want me to review again?

Athanaseus · 2025-01-13T11:31:12Z

@Athanaseus what's the status of the changes -- want me to review again?

Yes, it needs some final review. I added examples in the params info.
Now we can run with (we introduced the : as a separator for the coordinates string):
breizorro -r mypipelinerun-MFS-image.fits --merge mask.fits,adass.reg --number-islands --remove-islands 19,21,14h16m20.3:-65d42m07

o-smirnov · 2025-01-13T13:20:45Z

OK looks good, just one final suggestion -- in the yaml file, use dashes (i.e. remove-islands rather than remove_islands) in the names of the inputs. This is more consistent with the rest of our tooling (and with normal CLU conventions), and they'll get converted to underscores anyway when invoking the Python function.

Athanaseus added 30 commits March 14, 2024 16:07

Add cataloging routiine

b0f8048

updates

b3998b8

Restructuring and clean-ups

f904e45

Add utils and init file

4b48818

source detection updates

3ad1d58

Bump version

9b2bfb7

positionns

cf1e8e8

Refactor routines

d574bf3

Rename test_installation.yml to python-package.yml

ece367a

Create _config.yml

e8e6075

Update _config.yml

d6a9beb

Create index.md

e5798d2

Update index.md

1d182ae

Update README.rst

dd51336

Update index.md

e46f763

Update index.md

53e7408

Update index.md

896ce23

Update index.md

015dd70

Update README.rst

1b6a99d

Update index.md

64cbe40

Update index.md

f0daab2

Update _config.yml

975ff99

Update index.md

36a4e78

Update index.md

b36c3b3

Update index.md

660f5b1

legend color

83acfa1

Add files via upload

f1aa77e

Update index.md

bf9d06d

plot extent and sizing mode

337f692

Merge branch 'write_regions_catalogs' of https://github.com/ratt-ru/b…

76cba25

…reizorro into write_regions_catalogs

Athanaseus temporarily deployed to github-pages November 8, 2024 16:58 — with GitHub Pages Inactive

o-smirnov requested changes Dec 13, 2024

View reviewed changes

Athanaseus added 2 commits December 13, 2024 21:25

Update deps

db804fd

use clickify_parameters

56ffc96

Athanaseus temporarily deployed to github-pages December 13, 2024 19:27 — with GitHub Pages Inactive

Pin numpy to < 2 for older Python

19dbbec

Athanaseus temporarily deployed to github-pages December 13, 2024 19:38 — with GitHub Pages Inactive

test help command

438e8bd

Athanaseus temporarily deployed to github-pages December 13, 2024 19:42 — with GitHub Pages Inactive

update deps

7104796

Athanaseus temporarily deployed to github-pages December 13, 2024 19:49 — with GitHub Pages Inactive

exit 0

53fef7e

Athanaseus temporarily deployed to github-pages December 13, 2024 19:57 — with GitHub Pages Inactive

o-smirnov requested changes Dec 14, 2024

View reviewed changes

Athanaseus added 4 commits December 14, 2024 15:43

Upate config structure

285d453

Clean up

d32db93

add metavar

3d26852

Update repeat policies

866e08d

Athanaseus temporarily deployed to github-pages December 18, 2024 22:28 — with GitHub Pages Inactive

Update dtype and add examples

11d9211

Athanaseus temporarily deployed to github-pages December 19, 2024 23:06 — with GitHub Pages Inactive

Use rather than

4d0d609

Athanaseus temporarily deployed to github-pages January 13, 2025 14:28 — with GitHub Pages Inactive

o-smirnov approved these changes Jan 13, 2025

View reviewed changes

o-smirnov merged commit a936685 into main Jan 13, 2025
7 checks passed

o-smirnov deleted the write_regions_catalogs branch January 13, 2025 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write regions and catalogs #36

Write regions and catalogs #36

Athanaseus commented Nov 8, 2024 •

edited

Loading

o-smirnov Dec 13, 2024

o-smirnov Dec 13, 2024

o-smirnov Dec 13, 2024

o-smirnov commented Dec 13, 2024

o-smirnov Dec 14, 2024

o-smirnov Dec 14, 2024

o-smirnov Dec 14, 2024

o-smirnov Dec 14, 2024

o-smirnov Dec 14, 2024

Athanaseus Dec 14, 2024

o-smirnov Dec 17, 2024

Athanaseus Dec 18, 2024

o-smirnov Dec 19, 2024

o-smirnov Dec 19, 2024

Athanaseus Dec 19, 2024

o-smirnov Dec 14, 2024

o-smirnov Dec 14, 2024

o-smirnov commented Jan 13, 2025

Athanaseus commented Jan 13, 2025

o-smirnov commented Jan 13, 2025

Write regions and catalogs #36

Write regions and catalogs #36

Conversation

Athanaseus commented Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

o-smirnov commented Dec 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

o-smirnov commented Jan 13, 2025

Athanaseus commented Jan 13, 2025

o-smirnov commented Jan 13, 2025

Athanaseus commented Nov 8, 2024 •

edited

Loading