astropy / pyregion

ds9 region parser for python
https://pyregion.readthedocs.io
MIT License
39 stars 38 forks source link

cannot parse region file #73

Closed LejayChen closed 7 years ago

LejayChen commented 7 years ago

I am trying to overlay some regions on a plot by aplpy, which implement pyregion to do this, but pyregion failed to parse it (it works perfectly in ds9)

My region file looks like this: 'J2000; circle 188.5557102 12.0314056 1" # color=red'

when I run fig.show_regions('vcc1545.reg') after importing aplpy, It raises a ValueError which states: ValueError: need more than 0 values to unpack

and shows warnings like: UserWarning: Failed to parse : J2000; circle 188.5474559 12.0497103 1" # color=green

the source file that raises such error is pyregion/ds9_region_parser.py, I don't know where the problems are, thanks in advance for any kindly help

cdeil commented 7 years ago

@LejayChen - Thanks for the issue report.

I see the same behaviour:

>>> import pyregion
>>> pyregion.parse('J2000; circle 188.5557102 12.0314056 1" # color=red')
/Users/deil/code/pyregion/pyregion/ds9_region_parser.py:109: UserWarning: Failed to parse : J2000; circle 188.5557102 12.0314056 1" # color=red
  warnings.warn("Failed to parse : " + l)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/deil/code/pyregion/pyregion/core.py", line 187, in parse
    shape_list, comment_list = rp.filter_shape2(sss2)
ValueError: not enough values to unpack (expected 2, got 0)

The problem seems to be the double-quote character you use for the radius in arcsec. If I remove that character it works:

>>> pyregion.parse('J2000; circle 188.5557102 12.0314056 1 # color=red')
[Shape : circle ( Number(188.5557102),Number(12.0314056),Number(1) )]

PS: if I try the new regions package, parsing fails in a different way:

>>> import regions
>>> regions.ds9_string_to_objects('J2000; circle 188.5557102 12.0314056 1" # color=red')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/deil/Library/Python/3.5/lib/python/site-packages/regions-0.2.dev220-py3.5.egg/regions/io/read_ds9.py", line 219, in ds9_string_to_objects
    region_list = ds9_string_to_region_list(region_string)
  File "/Users/deil/Library/Python/3.5/lib/python/site-packages/regions-0.2.dev220-py3.5.egg/regions/io/read_ds9.py", line 252, in ds9_string_to_region_list
    parsed = line_parser(line, coordsys)
  File "/Users/deil/Library/Python/3.5/lib/python/site-packages/regions-0.2.dev220-py3.5.egg/regions/io/read_ds9.py", line 306, in line_parser
    raise ValueError("No coordinate system specified and a region has been found.")
ValueError: No coordinate system specified and a region has been found.

@keflavich or anyone that knows DS9 regions well: is this a valid region string? If yes, it's a parsing bug, right?

keflavich commented 7 years ago

@cdeil Good question. I'm not sure this is a valid region string; we tested all of the "official" test cases with astropy/regions, so at least a comparable region string is not included in those tests. I'm not sure if we have a complete specification to check this against, though. My suspicion is that this is not a valid region, but I know that " is acceptable as a unit specifier, so I am not certain.

LejayChen commented 7 years ago

After I change the line to 'J2000;circle(ra,dec,1") #color=red', it works.

For my original region string, at least I can load them in ds9.

keflavich commented 7 years ago

OK, so it looks like that original string should be supported, but isn't. We can add it to astropy/regions, but it's probably not worth backporting to pyregion.

cdeil commented 7 years ago

If ds9 parses it, and it's not too much work to adapt the parser to support it, I agree it would be good to add. (That said: I'm not volunteering to do it.)

LejayChen commented 7 years ago

Thanks @cdeil @keflavich , maybe I should turn to regions as well

keflavich commented 7 years ago

@LejayChen regions does not yet play with aplpy well, but that is one of the goals of that project. Stay tuned.

leejjoon commented 7 years ago

I guess the above PR fixes this problem. Please give it a try and let me know.

cdeil commented 7 years ago

Fixed by @leejjoon in #78 . Closing this now.

@keflavich - The DS9 parser in regions still doesn't work for that example. Do you have time to fix it, or alternatively, file a reminder issue there?

keflavich commented 7 years ago

No time now, but a reminder issue is important. Will file.