Repository navigation
Refactor Table readers and writers to use classes rather than free functions... #962
Description
...and generally make them more like the readers/writers in io.ascii.
{{ message }}
...and generally make them more like the readers/writers in io.ascii.
Activity
embray commentedon Apr 10, 2013
👍
astrofrog commentedon Apr 16, 2013
I agree this is a good idea - I won't have time to do this in the next couple of weeks, so if someone else wants to take a stab at it, feel free to!
astrofrog commentedon May 31, 2013
In terms of documentation, I'm starting to try and think how this could work, and I'm a bit worried we could end up with a monster docstring for
Table.read. I wonder whether we could have a docstring like:but I have trouble figuring out what
Xwould be (it could be something liket.read_<format>?which would be an auto-generated method but that could be confusing. Also not clear to me is the best way to present this on the website. Ultimately, we want people to be able to easily figure out the arguments needed for a specific format.taldcroft commentedon May 31, 2013
@astrofrog - I've thought about this a bit previously and the best I came up with was getting help programmatically via something like
Table.read(format='ascii', help=True). Even that requires that each connector instance (yes, we really need these to be classes now) has a customhelpmethod that knows about the underlying reader/writer and can fill in the right docstring on request.I just had another idea which is a little out there. What about making
readandwriteactually be callable class instances (where they do the usual thing when called), but with methods corresponding to the registered readers. ThenTable.read(format='fits', ...)would be the same asTable.read.fits(...), and from there you can have format-specific documentation in the docstring forTable.read.fits.astrofrog commentedon May 31, 2013
@taldcroft - these are good ideas. One could also have
Table.read_help('fits')for the first since it's not clear otherwise whether you mean it should read it anyway, or if it will just show the docstring.For the second case, you could consider
Table.fits.readin which casefitsis the class we are talking about.taldcroft commentedon May 31, 2013
I don't think I would want to clutter the Table attribute space with all the readers, which will include obscure ones like
fixed_width_two_line.astrofrog commentedon May 3, 2014
I'm re-scheduling this and #2063 to 1.0 since I think it will require some more thought/work.
taldcroft commentedon May 3, 2014
Should we put in #1888 in the meantime then?
astrofrog commentedon May 3, 2014
@taldcroft - yes, that would probably be reasonable. Is #1888 ready for final review in that case?
taldcroft commentedon May 3, 2014
Yes, ready for final review.
2 remaining items
astrofrog commentedon Sep 29, 2015
Removing milestone since not 1.1-critical
bsipocz commentedon Jun 19, 2019
Can we close this now?
astropy-bot commentedon Jun 26, 2019
Hi humans 👋 - this issue was labeled as Close? approximately 7 days ago. If you think this issue should not be closed, a maintainer should remove the Close? label - otherwise, I will close this issue in a month.
If you believe I commented on this issue incorrectly, please report this here
astropy-bot commentedon Jul 29, 2019
I'm going to close this issue as per my previous message, but if you feel that this issue should stay open, then feel free to re-open and remove the Close? label.
If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here