Merge 605fb7a into db84b63

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(
@@ -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_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.lower()])
 
             if 'comment' in shape.meta:
                 meta_str += ", " + shape.meta['comment']

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]

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 ....shapes.ellipse 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):

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)