Small clean-ups of IERS table reader - #3223

👍

@eteq - I got around to this refactoring of iers. Could you have a look? One thing that would be nice is some tests of the PM_x, PM_y stuff in iers/test_iers -- though I guess it does get exercised through coordinates.

@taldcroft - since this touches Time, might you also be able to have a quick look? Note how iers now is a QTable -- lovely with those Quantity columns (though it also is clear that it would be even nicer if we had masked quantities; but remains annoyingly tricky without numpy changes...).

Inspired by the coverage failure, I added quite a few more tests, including at least some basic ones on the polar motion. @eteq - not knowing anyone more suitable, I assigned this to you... Could you have a look? I think it is really quite straightforward, but good to have a second set of eyes.

Note: the appveyor failure is just a time-out.

@mhvk - my only question is whether the change from Table to QTable represents a user API change. From my understanding the IERS table is internal, but just checking.

Otherwise I am hoping to look at some other PRs with my name on them and leave this to @eteq.

@taldcroft - this is mostly for internal use, but you're right that it still is a bit of an API change, in particular in IERS.ut1_utc now returning a Quantity. I've added an entry to CHANGES.rst and will change this to affects-release.

@eteq - I rebased this IERS refactor to ensure it still passed all tests. Fortunately, it does. Will you have time to look at it? I'm quite sure it is OK, since really it is just reorganisation...

@eteq - would you have a chance to review? I think this is quite ready to go in, and I'd rather do it ahead of some other changes being made to Time.

@eteq - would you be able to have a look soon? If not, could you ask someone else?

@taldcroft - thanks, corrected (and rebased, just to be sure).

@mhvk - I took an inexpert look at the code and didn't see anything obviously wrong. I looked slightly more closely at the new / updated tests and it all seems reasonable. 👍 from my perspective.

I personally think that all API changes should go into the What's New document (under backward-incompatible changes), so they get proper visibility. In this case there probably aren't a huge number of IERS table users.

@taldcroft - I looked again at the CHANGES.rst and I think the API changes section is a bit more logical, especially as this really is a minimal change, just making the output consistent between time offset and polar motion. I'll go ahead and merge so I can continue on Time shape changes, etc.

@mhvk - sorry, didn't mean to imply taking it out of CHANGES.rst, but as a reminder to also put things like this in a more visible place for the release. But that is really a different discussion.

OK, no problem.

@mhvk @taldcroft - sorry I never got to reviewing this - but the good news is that I "post-reviewed" it and saw no problems!

Also, @bmorris3, you might want to be aware of this in case you've started some IERS work on a branch that was before this was merged (2 days ago).

@eteq - thanks for the post-review!