Repository navigation
Easier subclassing for Table - #1888
Conversation
|
This might be related to #1833, #1835, and similar--though those have more to do with subclassing I see the problem though. I think any "registering" of subclasses should happen automatically, and need not be explicit as in your second suggestion. Something inbetween 1 or 2 might be best. |
|
#1046 also addresses this problem. |
|
@embray - I think #1046 is most relevant, but broader than the problem raised by @joezuntz. I ran into it as well when writing the IERS classes [1]. I'd think this should not really be that hard, but the I/O parts are among those I do not really understand how they work. @taldcroft? @joezuntz - maybe you could provide a link to your code, just to get a sense of what you're thinking of? |
|
Sure - see here: This creates a subclass of Table called I3Table. The first two methods (specials getattr and repr) were just my original motivation for making the subclass, so you can ignore those. The last two do the work of relevance here. |
|
I would definitely prefer a solution like #1 where subclasses just work by default for I/O. It seems like that should be relatively straightforward, at least to the extent that they don't do anything very crazy, but I haven't looked at the code carefully with this in mind. I can also imagine that some sort of |
|
We could use a metaclass for auto-registration (as in |
As documented [here](astropy/astropy#1888) there are currently issues with subclasses of Table since they are now grandfathered into the astropy I/O registry. Here I've applied the workaround advocated in [this gist](https://gist.github.com/joezuntz/7929541). A test of writing and reading a PhotTable as an HDF5 file is included.
|
I've attached some code with a possible implementation of idea (1) to allow subclasses to use the parent class read/write functions. This SHOULD NOT BE MERGED without resolution of #2063, but may serve as guidance for putting this functionality there. |
|
@taldcroft - since #2063 has been delayed, do we merge this now, or also delay to 1.0? |
|
I was hoping to merge this now. That will levy an additional requirement on refactoring (e.g. #2063) that it support subclasses in a compatible way, but that seems reasonable to me. |
|
Looks good to me! |
Currently if you subclass table it breaks the I/O methods, because the registry dictionaries do not contain entries for the subclass, only the parent. This makes it quite fiddly to subclass.
e.g.
will raise an exception "No reader defined for format 'ascii' and class 'MyTable'.".
Sometimes this might be necessary, if the subclass alters the structure of the class enough that the original functions are invalid, but most subclasses would not have this problem.
I can think of two ways to make this easier:
I have a working version of the latter that I use for my own code, and if it would be useful I can do a PR.
Thanks! Joe Zuntz