From bc645192cadece4549694a6b02533582f1b1f3ca Mon Sep 17 00:00:00 2001 From: "Adam Ginsburg (keflavich)" Date: Thu, 14 May 2020 09:32:14 -0400 Subject: [PATCH 1/7] fix for issue 312 --- regions/io/core.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/regions/io/core.py b/regions/io/core.py index 4443f060..39bee16c 100644 --- a/regions/io/core.py +++ b/regions/io/core.py @@ -173,15 +173,19 @@ def to_crtf(self, coordsys='fk5', fmt='.6f', radunit='deg'): if shape.meta.get('label', "") != "": shape.meta['label'] = "'{}'".format(shape.meta['label']) meta_str = ", ".join(f"{key}={val}" for key, val in - shape.meta.items() if - key not in ('include', 'comment', 'symbol', - 'coord', 'text', 'range', 'corr', - 'type')) + shape.meta.items() if + key not in ('include', 'comment', 'symbol', + 'coord', 'text', 'range', 'corr', + 'type')) # the first item should be the coordinates, since CASA cannot # recognize a region without an inline coordinate specification # It can be, but does not need to be, comma-separated at the start - meta_str = "coord={}, ".format(coordsys.upper()) + meta_str + if meta_str.strip(): + meta_str = "coord={}, ".format(coordsys.upper()) + meta_str + else: + # if there is no metadata at all (above), the trailing comma is incorrect + meta_str = "coord={}".format(coordsys.upper()) if 'comment' in shape.meta: meta_str += ", " + shape.meta['comment'] From 85a219400a8e8cae527ee679ff13befd51ac1ce5 Mon Sep 17 00:00:00 2001 From: "Adam Ginsburg (keflavich)" Date: Thu, 14 May 2020 09:39:06 -0400 Subject: [PATCH 2/7] add regression test --- regions/io/crtf/tests/test_crtf_language.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/regions/io/crtf/tests/test_crtf_language.py b/regions/io/crtf/tests/test_crtf_language.py index 3e8d327a..d7b15de0 100644 --- a/regions/io/crtf/tests/test_crtf_language.py +++ b/regions/io/crtf/tests/test_crtf_language.py @@ -5,11 +5,15 @@ import astropy.version as astrov from astropy.utils.data import get_pkg_data_filename +from astropy import coordinates, units as u + from ..read import CRTFParser, read_crtf from ..write import crtf_objects_to_string from ..core import CRTFRegionParserError +from ..core import EllipseSkyRegion + _ASTROPY_MINVERSION = vers.LooseVersion('1.1') _ASTROPY_VERSION = vers.LooseVersion(astrov.version) @@ -123,6 +127,18 @@ def test_valid_region_syntax(): assert "('3arcmin', '') should be a pair of length" in str(excinfo.value) +def test_issue_312_regression(): + """ + Make sure there is no trailing comma when writing a CRTF string with no metadata + """ + reg = EllipseSkyRegion(center=coordinates.SkyCoord(279.174990*u.deg, + -21.257123*u.deg, + frame='fk5'), + width=0.001571*u.deg, height=0.001973*u.deg, + angle=111.273322*u.deg) + crtfstr = crtf_objects_to_string([reg], 'fk5', '.6f', 'deg') + assert crtfstr.strip()[-1] != ',' + @pytest.mark.parametrize('filename', ['data/CRTFgeneral.crtf', 'data/CRTFgeneraloutput.crtf']) def test_file_crtf(filename): From 477d1877f88e5b83caa947e5e898466724cfc2a0 Mon Sep 17 00:00:00 2001 From: "Adam Ginsburg (keflavich)" Date: Thu, 14 May 2020 09:52:19 -0400 Subject: [PATCH 3/7] fix import --- regions/io/crtf/tests/test_crtf_language.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regions/io/crtf/tests/test_crtf_language.py b/regions/io/crtf/tests/test_crtf_language.py index d7b15de0..b709959d 100644 --- a/regions/io/crtf/tests/test_crtf_language.py +++ b/regions/io/crtf/tests/test_crtf_language.py @@ -12,7 +12,7 @@ from ..write import crtf_objects_to_string from ..core import CRTFRegionParserError -from ..core import EllipseSkyRegion +from ....shapes.ellipse import EllipseSkyRegion _ASTROPY_MINVERSION = vers.LooseVersion('1.1') _ASTROPY_VERSION = vers.LooseVersion(astrov.version) From 4356b812e53aa82ae551fe9297645801aaa9c01d Mon Sep 17 00:00:00 2001 From: "Adam Ginsburg (keflavich)" Date: Thu, 14 May 2020 09:59:14 -0400 Subject: [PATCH 4/7] fix docstring: no trailling comma! --- regions/io/crtf/write.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regions/io/crtf/write.py b/regions/io/crtf/write.py index 8843bcdc..0d037824 100644 --- a/regions/io/crtf/write.py +++ b/regions/io/crtf/write.py @@ -37,7 +37,7 @@ def crtf_objects_to_string(regions, coordsys='fk5', fmt='.6f', radunit='deg'): >>> reg_sky = CircleSkyRegion(SkyCoord(1 * u.deg, 2 * u.deg), 5 * u.deg) >>> print(crtf_objects_to_string([reg_sky])) #CRTF - circle[[1.000007deg, 2.000002deg], 5.000000deg], coord=FK5, + circle[[1.000007deg, 2.000002deg], 5.000000deg], coord=FK5 """ shapelist = to_shape_list(regions, coordsys) From 5c95c0f4cca11b6e4902fa528a3fe5c924f624ea Mon Sep 17 00:00:00 2001 From: "Adam Ginsburg (keflavich)" Date: Fri, 15 May 2020 08:21:10 -0400 Subject: [PATCH 5/7] implement @JonahDW's suggestion --- regions/io/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/regions/io/core.py b/regions/io/core.py index 39bee16c..e19cb225 100644 --- a/regions/io/core.py +++ b/regions/io/core.py @@ -182,10 +182,10 @@ def to_crtf(self, coordsys='fk5', fmt='.6f', radunit='deg'): # recognize a region without an inline coordinate specification # It can be, but does not need to be, comma-separated at the start if meta_str.strip(): - meta_str = "coord={}, ".format(coordsys.upper()) + meta_str + meta_str = "coord={}, ".format(coordsys_mapping['CRTF'][coordsys]) + meta_str else: # if there is no metadata at all (above), the trailing comma is incorrect - meta_str = "coord={}".format(coordsys.upper()) + meta_str = "coord={}".format(coordsys_mapping['CRTF'][coordsys]) if 'comment' in shape.meta: meta_str += ", " + shape.meta['comment'] From b11f1dc184289c01bb48a0b579b0ee8fb296aea1 Mon Sep 17 00:00:00 2001 From: "Adam Ginsburg (keflavich)" Date: Fri, 15 May 2020 11:32:26 -0400 Subject: [PATCH 6/7] try more fixes of mappings: caught an error where arcsecond units were applied in questionable ways (this is starting to overlap w/Jonah's work) --- regions/io/core.py | 18 +++++++++--------- regions/io/crtf/write.py | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/regions/io/core.py b/regions/io/core.py index e19cb225..9aa06c20 100644 --- a/regions/io/core.py +++ b/regions/io/core.py @@ -69,12 +69,12 @@ class RegionConversionError(ValueError): # Maps astropy's coordinate frame names with their respective name in the file format. coordsys_mapping = {'DS9': {x: x for x in valid_coordsys['DS9']}, - 'CRTF': {x: x for x in valid_coordsys['CRTF']} + 'CRTF': {x: x.upper() for x in valid_coordsys['CRTF']} } -coordsys_mapping['CRTF']['geocentrictrueecliptic'] = 'ecliptic' -coordsys_mapping['CRTF']['fk5'] = 'j2000' -coordsys_mapping['CRTF']['fk4'] = 'b1950' -coordsys_mapping['CRTF']['supergalactic'] = 'supergal' +coordsys_mapping['CRTF']['geocentrictrueecliptic'] = 'ECLIPTIC' +coordsys_mapping['CRTF']['fk5'] = 'J2000' +coordsys_mapping['CRTF']['fk4'] = 'B1950' +coordsys_mapping['CRTF']['supergalactic'] = 'SUPERGAL' coordsys_mapping['DS9']['geocentrictrueecliptic'] = 'ecliptic' @@ -140,8 +140,8 @@ def to_crtf(self, coordsys='fk5', fmt='.6f', radunit='deg'): output = '#CRTF\n' if radunit == 'arcsec': - # what's this for? - if coordsys in coordsys_mapping['CRTF'].values(): + # arcseconds are allowed for all but image coordinates + if coordsys.lower() not in ('image',): radunitstr = '"' else: raise ValueError( @@ -182,10 +182,10 @@ def to_crtf(self, coordsys='fk5', fmt='.6f', radunit='deg'): # recognize a region without an inline coordinate specification # It can be, but does not need to be, comma-separated at the start if meta_str.strip(): - meta_str = "coord={}, ".format(coordsys_mapping['CRTF'][coordsys]) + meta_str + meta_str = "coord={}, ".format(coordsys_mapping['CRTF'][coordsys.lower()]) + meta_str else: # if there is no metadata at all (above), the trailing comma is incorrect - meta_str = "coord={}".format(coordsys_mapping['CRTF'][coordsys]) + meta_str = "coord={}".format(coordsys_mapping['CRTF'][coordsys.lower()]) if 'comment' in shape.meta: meta_str += ", " + shape.meta['comment'] diff --git a/regions/io/crtf/write.py b/regions/io/crtf/write.py index 0d037824..e90eb2c6 100644 --- a/regions/io/crtf/write.py +++ b/regions/io/crtf/write.py @@ -17,7 +17,7 @@ def crtf_objects_to_string(regions, coordsys='fk5', fmt='.6f', radunit='deg'): List of `~regions.Region` objects coordsys : `str`, optional Astropy Coordinate system that overrides the coordinate system frame for - all regions. Default is 'fk5'. + all regions. Default is 'fk5' (which maps to J2000). fmt : `str`, optional A python string format defining the output precision. Default is .6f, which is accurate to 0.0036 arcseconds. @@ -37,7 +37,7 @@ def crtf_objects_to_string(regions, coordsys='fk5', fmt='.6f', radunit='deg'): >>> reg_sky = CircleSkyRegion(SkyCoord(1 * u.deg, 2 * u.deg), 5 * u.deg) >>> print(crtf_objects_to_string([reg_sky])) #CRTF - circle[[1.000007deg, 2.000002deg], 5.000000deg], coord=FK5 + circle[[1.000007deg, 2.000002deg], 5.000000deg], coord=J2000 """ shapelist = to_shape_list(regions, coordsys) From 605fb7a2927d280207b7df4f834a24ce2143b179 Mon Sep 17 00:00:00 2001 From: "Adam Ginsburg (keflavich)" Date: Fri, 15 May 2020 12:02:55 -0400 Subject: [PATCH 7/7] fix output test data --- regions/io/crtf/tests/data/CRTFgeneraloutput.crtf | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/regions/io/crtf/tests/data/CRTFgeneraloutput.crtf b/regions/io/crtf/tests/data/CRTFgeneraloutput.crtf index 75cfb8e3..c0dcd3cd 100644 --- a/regions/io/crtf/tests/data/CRTFgeneraloutput.crtf +++ b/regions/io/crtf/tests/data/CRTFgeneraloutput.crtf @@ -1,8 +1,8 @@ #CRTF -ann circle[[273.100deg, -23.183deg], 0.001deg], coord=FK4, frame=BARY, color=blue, corr=[I, Q] -rotbox[[180.392deg, 12.392deg], [0.050deg, 0.017deg], 12.000deg], coord=FK4, frame=BARY, color=blue, range=[-1240.0km/s, 1240.0km/s], corr=[I, Q] -annulus[[267.763deg, -45.297deg], [0.100deg, 4.120deg]], coord=FK4, label='My label here', frame=BARY, color=red, corr=[I, Q, U, V] --ellipse[[12.000deg, 45.000deg], [0.250deg, 1.340deg], 2578.310deg], coord=FK4, label='Removed this', frame=BARY, color=green, range=[1.42GHz, 1.421GHz], corr=[Q] -symbol[[31.470deg, 11.905deg], D], coord=FK4, frame=BARY, color=blue, linewidth=2, symsize=2, corr=[I, Q] -text[[31.470deg, 11.905deg], 'my text'], coord=FK4, frame=BARY, color=blue, linewidth=2, corr=[I, Q] -rotbox[[2.000deg, 3.000deg], [2.000deg, 2.000deg], 0.000deg], coord=FK4, frame=BARY, color=blue, corr=[I, Q] +ann circle[[273.100deg, -23.183deg], 0.001deg], coord=B1950, frame=BARY, color=blue, corr=[I, Q] +rotbox[[180.392deg, 12.392deg], [0.050deg, 0.017deg], 12.000deg], coord=B1950, frame=BARY, color=blue, range=[-1240.0km/s, 1240.0km/s], corr=[I, Q] +annulus[[267.763deg, -45.297deg], [0.100deg, 4.120deg]], coord=B1950, label='My label here', frame=BARY, color=red, corr=[I, Q, U, V] +-ellipse[[12.000deg, 45.000deg], [0.250deg, 1.340deg], 2578.310deg], coord=B1950, label='Removed this', frame=BARY, color=green, range=[1.42GHz, 1.421GHz], corr=[Q] +symbol[[31.470deg, 11.905deg], D], coord=B1950, frame=BARY, color=blue, linewidth=2, symsize=2, corr=[I, Q] +text[[31.470deg, 11.905deg], 'my text'], coord=B1950, frame=BARY, color=blue, linewidth=2, corr=[I, Q] +rotbox[[2.000deg, 3.000deg], [2.000deg, 2.000deg], 0.000deg], coord=B1950, frame=BARY, color=blue, corr=[I, Q]