Use descriptors for frame attributes - #2626

@eteq - previous test fails were unrelated, but I had to rebase. I just added some unit tests of FrameAttribute which should test and demonstrate the slightly intricate logic relating to default handling. I feel better now with some tests. :-)

@taldcroft - I was just about to point out the Example -> Examples and need to add it to __all__... so you read my mind!

So now this looks good except for three things:

• Not using the MRO to get the attribute names is a problem, IMHO. Note the following behavior:

>>> class SubFK5(FK5):
>>>     pass
>>> c = SubFK5()
>>> c
<SubFK5 Frame>
>>> c.equinox
<Time object: scale='utc' format='jyear_str' value=J2000.000>
>>> c.frame_attr_names()
{}

That's confusing: the equinox attribute exists because it's in the superclass, but it's not settable or visible in frame_attr_names. So I would prefer if you put back the MRO-searching code when populating frame_attr_names. As you say, it doesn't matter right now because there are no sub-class frames, but I think consistency now is important so as not to have to break it in future versions. Also, a PR I'm working on now will probably need the MRO-searching stuff to stay in.

• The machinery to do the conversion from strings like 'J2000' to Time objects still lives in BaseCoordinateFrame. It might make more sense to add a convert_input method to FrameAttribute that can be used in BaseCoordinateFrame, and then also add a TimeFrameAttribute subclass that just overrides convert_input. That would allow user-defined FrameAttributes that do any arbitrary sort of conversions (which I'd love, because the time_attr_names grated on me as I wrote it, as it's not as extensible as I would have liked)
• I think it would be better to rename frame_attr_names to get_frame_attr_names so that it's clear it's now a method, not an attribute.

@eteq - almost there with getting rid of time_attr_name (agreed this is a very good thing). Something is still messed up but I have to stop. If you have time check out the commit here : taldcroft@b49b0c8.

Also: https://github.com/taldcroft/astropy/compare/coordinates-generic-frame-attr

I hadn't looked too closely at much of the coordinates code yet, but this looks like a much nicer solution than what was already there.

@taldcroft - if it helps any, I cleaned up the MRO stuff some in #2629 , so that might help here...

@eteq - I got stuck in a deep rabbit hole on this but finally dug my way out. This is passing tests locally and as far as I know the documentation updates are complete. Some of the logic is more intricate than I'd like but I didn't find any ways to simplify further.

OK, tests passing and ready for final review. I milestoned this for 0.4.

Looks good to merge to me, @taldcroft, aside from the minor comment and question above (which may require no action depending on the answer).

@eteq - Now we have a fully working example with doctest coverage. Much better! And I think this is ready for merge now? The getattr issue is, in any case, an implementation detail that we don't have to resolve right now.

@eteq - ball in your court now. I think this is done.

@taldcroft - one very minor remaining doc comment, but the code itself looks good to me. If you haven't gotten to this in a little while, I'll probably just merge this so that I can get to rebasing #2629 , and the docfix can go in a separate PR or something.

@eteq - I'm just going to merge now to save waiting for Travis (which would be formality, but still...). We can fix it any time, maybe even in #2629. I just didn't know the right format for multiple outputs.

@taldcroft - cool - I'll probably fix it in #2629 then, or possibly as a separate minor PR.