diff --git a/CHANGES.rst b/CHANGES.rst index 183eca44..c58377b9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,9 @@ New Features Bug Fixes --------- +- Fixed a doubled comma in the serialized CRTF output when ``range`` or + ``corr`` is the only metadata of a region. [#700] + API Changes ----------- diff --git a/regions/io/crtf/io_core.py b/regions/io/crtf/io_core.py index 720cce20..52e202a0 100644 --- a/regions/io/crtf/io_core.py +++ b/regions/io/crtf/io_core.py @@ -183,7 +183,6 @@ def to_crtf(self, coordsys='fk5', fmt='.6f', radunit='deg'): for key, val in shape.meta.items(): if key not in keylist: meta_pairs.append(f'{key}={val}') - meta_str = ', '.join(meta_pairs) # The first item should be the coordinates, since CASA # cannot recognize a region without an inline coordinate @@ -192,22 +191,23 @@ def to_crtf(self, coordsys='fk5', fmt='.6f', radunit='deg'): shape_coordsys = shape.coordsys if shape_coordsys.lower() != coordsys.lower(): coord = coordsys_mapping['CRTF'][coordsys.lower()] - if meta_str.strip(): - meta_str = f'coord={coord}, ' + meta_str - else: - # if there is no metadata at all (above), the - # trailing comma is incorrect - meta_str = f'coord={coord}' + meta_pairs.insert(0, f'coord={coord}') if 'comment' in shape.meta: - meta_str += ', ' + shape.meta['comment'] + meta_pairs.append(shape.meta['comment']) if 'range' in shape.meta: shape.meta['range'] = [str(str(x).replace(' ', '')) for x in shape.meta['range']] - meta_str += f", range={shape.meta['range']}".replace("'", '') + meta_pairs.append( + f"range={shape.meta['range']}".replace("'", '')) if 'corr' in shape.meta: - meta_str += f", corr={shape.meta['corr']}".replace("'", '') + meta_pairs.append( + f"corr={shape.meta['corr']}".replace("'", '')) + + # join only once so that an empty item cannot leave a + # doubled comma in the output (issue #322) + meta_str = ', '.join(meta_pairs) coord = [] if coordsys not in ['image', 'physical']: diff --git a/regions/io/crtf/tests/test_crtf.py b/regions/io/crtf/tests/test_crtf.py index 62c3a950..ea0e2b8c 100644 --- a/regions/io/crtf/tests/test_crtf.py +++ b/regions/io/crtf/tests/test_crtf.py @@ -11,7 +11,7 @@ from astropy.tests.helper import assert_quantity_allclose from astropy.utils.data import get_pkg_data_filename -from regions.core import Regions +from regions.core import RegionMeta, Regions from regions.io.crtf.core import CRTFRegionParserError from regions.io.crtf.read import _CRTFParser from regions.shapes.circle import CircleSkyRegion @@ -124,6 +124,25 @@ def test_issue_312_regression(): assert crtfstr.strip()[-1] != ',' +@pytest.mark.parametrize(('meta', 'expected'), + [({'corr': ['XX']}, 'deg], corr=[XX]'), + ({'range': ['1000MHz', '2000MHz']}, + 'deg], range=[1000MHz, 2000MHz]')]) +def test_issue_322_regression(meta, expected): + """ + Make sure there is no doubled comma when writing a CRTF string + where range or corr is the only metadata. + """ + reg = EllipseSkyRegion(center=SkyCoord(279.486483 * u.deg, + -20.683327 * u.deg, frame='fk5'), + width=0.009218 * u.deg, height=0.005954 * u.deg, + angle=100.410911 * u.deg, meta=RegionMeta(meta)) + crtfstr = reg.serialize(format='crtf', coordsys='fk5', fmt='.6f', + radunit='deg') + assert ', ,' not in crtfstr + assert expected in crtfstr + + @pytest.mark.parametrize(('filename', 'outname', 'coordsys', 'fmt'), [('data/CRTFgeneral.crtf', 'data/CRTFgeneraloutput.crtf',