Proposal for new I/O API

[larrybradley]

The regions.io package seems to have grown organically, leading to some API inconsistency (e.g., see #375 and #376) and awkwardness (e.g., non-obvious function names like fits_region_objects_to_table, which actually converts a list of regions to a Table). I propose to greatly simplify the I/O API with the following interface:

• Reading region files -- input a filename, return a RegionList object (this is a new class)
  • read_ds9, read_crtf, read_fits
  • Writing region files -- input a RegionList (or list of regions) and filename
  • write_ds9, write_crtf, write_fits
  • Parsing region strings -- input a region string (or table), return a RegionList object
  • parse_ds9, parse_crtf, parse_fits
  • Serialization of a RegionList object to a string (or table) with methods:
  • to_ds9_string, to_crtf_string, to_fits_table (or perhaps just to_ds9, to_crtf, to_fits?)
  • The new RegionList class would support the unified I/O interface with read and write methods
  • The new RegionList class could also have the parsing class methods (parse_ds9, parse_crtf, parse_fits)

This simplified API allows us to eventually remove (after a deprecation period):

• ShapeList and Shape objects -- these classes are really just intermediate helpers when parsing region files with the *Parser classes. In all cases that I could find, they are immediately converted into a list of regions (e.g., regions.CRTFParser(crtf_region).shapes.to_regions()). The Shape* objects are never directly used. All of the functionality of these classes would be in the Region and new RegionList objects.
• The parser classes: DS9Parser, CRTFParser, FITSRegionParser. These classes would be replaced by the parser functions listed above (and RegionList class methods).
• The serialization functions: ds9_objects_to_string, crtf_objects_to_string, fits_region_objects_to_table. These functions would be replaced by the RegionList serialization methods above.

This can all be done without any breaking changes. There would be a very minor API change for the read functions because they would now return a RegionList object instead of list of regions, but the RegionList object effectively would behave exactly like a list of regions. For the write functions, we could still allow the input to be a list of regions (but converted internally to a RegionList object).

@keflavich (and perhaps @astrofrog) -- Before I proceed with all this work, I'd like to hear your thoughts. Please let me know soon as I'd like to get this in the upcoming 0.5 release (to start the deprecation period).

EDIT: These API issues go back to #274

[keflavich]

This looks fine to me, at least on the surface. So :+1:

[astrofrog]

Regarding:

Reading region files -- input a filename, return a RegionList object (this is a new class)
read_ds9, read_crtf, read_fits

is there a reason we can't instead use the unified I/O approach of having e.g.

RegionList.read('...', format='ds9')
RegionList.read('...', format='crtf')
RegionList.read('...', format='fits')

?

(same for writing)

[larrybradley]

The RegionList will use the unified I/O approach in this plan. I'm fine with removing the separate read/write functions -- they already exist, so they will need to be deprecated (and supported for a while). But @keflavich should definitely comment if it's OK to deprecate and eventually remove the read/write functions.

In that case, the separate parser functions should probably also be removed and they should only be RegionList methods.

[keflavich]

if we're going to remove the existing readers, making a major API change, we definitely need to go through a deprecation period first. But this is probably the right way to go, so, yes, go ahead and deprecate them

[larrybradley]

OK, will do.

[astrofrog]

I don't feel strongly about removing the individual functions - no harm in keeping them if it would cause major breakage. But we could advertise RegionList.read and RegionList.write as the main API.