Add format arg to io.ascii read/write and overhaul io.ascii.connect#961
Conversation
|
This is the first time I've really looked at the OT, but would it make sense to update the table readers to work in a similar way, i.e. to use classes with special attributes rather than free-floating functions? |
|
@mdboom - yeah, I can imagine it might clean things up to have some sort of IO connector class that encapsulates the read, write, identifier functions, handles aliases, suffixes and whatnot. |
|
Ok -- obviously not urgent, but I open #962 for that idea. |
|
This looks like a good idea.
|
|
@hamogu - I'm not comfortable with "hiding" some readers. I find the basic reader works for me at best ~50% of the time. A lot of tables in the wild (at least, in my sub-fields) seem to be imporoperly formatted with spaces mixed with tabs or headers with spaces, or the like. In that case I usually need one of the specialized ones. But you are right that the basic one is still most common - so I guess mainly what I'm suggesting is instead "Reading tries all formats automatically, these are the most common cases for writing: ascii, aastex, fixed_width, latex, noheader and see the table below (but on this page) for the full listing". @taldcroft - one of the concerns I have with this is that it makes it harder to find the appropriate reader/writer class, and hence the relevant specific documentation. That's one thing that was nice about |
|
One disadvantage of this scheme is that is is harder to subclass a Reader or Writer and use it. Before, I just make a Reader and pass it with |
|
Ah, but you didn't read the fine print. :-) By use of metaclasses, whenever you inherit from |
|
That addresses my concern! |
|
I wonder whether in the long term, we should have a |
|
I would go one level further and say that there should be a |
|
@taldcroft - ah yes of course, the I/O infrastructure is also there for |
|
Closes #385. |
|
@hamogu @astrofrog @eteq @mdboom This one got stuck for a while but I think it's ready now (at least from my perspective). I did a significant rework of what was there before and now have (nominally) written the required tests and done all the doc updates. Although not strictly part of the PR to add a format arg to ascii.read/write, I ended doing a bunch of reworking the unified interface documentation since I discovered big chunks that were redundant. Preview of docs at: http://hea-www.harvard.edu/~aldcroft/tmp/astropy-format/html/index.html I see now there is a merge conflict again, sigh. (Third time now that I have had to rebase this branch because of conflicts). |
* 95592e1 Fix multiple registration problem for aliased formats * 0889189 Redo the auto I/O registration in a more consistent way * 178a2e4 Add table showing available ASCII I/O connector formats * 331a69f Add auto-registration of readers/writers (PARTIAL) * d0841b1 Add format keyword to ascii.read and write methods
|
Rebased and fixed merge conflict in io/ascii/write.rst documentation. |
|
Looks great, thanks for working on this! The documentation is already much improved with this, though I think we can try and do even better once we work on #962 - I think that ideally, someone using only the unified I/O shouldn't really have to go to the docstring of the e.g. and I have no idea what options can be passed for reading (I just noticed that the Before this can be merged: Failed test due to the use of |
|
Yes, the online docs situation needs work, but as you said that's out of scope for this PR. I did just add explicit references to the io.ascii read and write sections that provide long descriptions of every parameter, so if anyone is reading the web docs they'll have a reasonably clear idea of what's available: http://hea-www.harvard.edu/~aldcroft/tmp/astropy-format/html/io/unified.html#ascii-formats |
|
OK, travis is passing now. |
|
Ok - this looks good to me! Feel free to merge (unless you want to wait for someone else to review too) |
Add format arg to io.ascii read/write and overhaul io.ascii.connect
|
@taldcroft - this looks great. I agree with @astrofrog's comments about clearing up the online and interactive docs, but that is indeed a separate issue. FYI, I noticed a whitespace typo in the docs that was mucking up the format of the formats table, which I fixed directly in 374883d |
This PR makes
io.ascii.read() / write()more consistent with other parts ofastropyby adding aformatkeyword (as a string) and deprecating theReaderandWriterkeywords (which accept only anio.asciireader classes).In conjunction the I/O registration of
io.asciireaders/writers has been automated so that every format class automatically registers via a base metaclass and class attributes. This removed a bunch of boilerplate code inio.connect. There is a new routine that (mostly) generates the sphinx table output which summarizes the IO registry ascii formats that are available.I'm putting this PR out before it is fully baked in order to ask for feedback on the format names for the IO registry. Going in to this I was hesitant to about putting all of the ASCII formats into the registry with their native format name. For instance, with no context this would seem odd to me:
With that in mind I made every ASCII format in the registry default to being prefixed by
ascii:, so it would beOf course some of the formats do make sense in a standalone fashion, like
This is required anyway for back-compatibility with 0.2. But now when I look at the registry table it all looks a bit messy. So this is a call for advice / opinions about how to make this tidy.
cc: @hamogu