p.s. Appveyor is already using numpy "stable", so that is fine. It didn't fail, since it is not building docs.

I have a deja vu that you already had a PR for this.

#4456 :)

@astrofrog - #4456 is a bit larger, moving everything to 3.5. That one was failing the sphinx build, and I noticed this was a more general problem with numpy 1.10. So I thought I should address that separately. I think this one does it, but wonder whether I should add a more elaborate plotting test, that uses fill_between as in the observing example, so this doesn't recur. What do you think?
(In the meantime, I'll rebase #4456 on this work, so see if that now passes...)

@mhvk - interesting. I hadn't realized there was a regression in numpy v1.10 for this.

But I find it rather awkward that you need quantity_support() for this to work. I wonder if this is a call for quantity_support() to be default behavior? Probably better to merge this in its current form and consider that separately, but food for thought...

And shouldn't this be 1.1.x? That is, this is a doc clarification and a bug fix, right? That is, quantity_support is in 1.1. Or is this affecting some new aspect of it?

Another consideration: this means that astropy v1.0.x/numpy 1.10 is broken, right? Perhaps we should instead fix this example by adding .value to the times in the fill_between call, and have that get backported all the way to 1.0.x? It looks to me like that works even with numpy 1.10.

Clarification: I'm suggesting the .value option because this example is already written assuming the "old" way of not having quantity support, so it has to set the labels manually. Doesn't really matter that much if it also does the .value in that case.

@eteq - you're right, since quantity_support is in 1.1, this is a bug fix. I think it may be easiest if I just separate it out as a separate bug. With that, I think the documentation update here is OK for 1.1 and onwards, but for 1.0 it would need to use .value. At least, I think it is better not to use .value generally, but rather show the users how to work with quantities also in plotting. It is a good question of whether quantity_support should be on by default, but maybe we can leave that for a separate issue?

since quantity_support is in 1.1, this is a bug fix. I think it may be easiest if I just separate it out as a separate bug.

👍

At least, I think it is better not to use .value generally, but rather show the users how to work with quantities also in plotting.

I agree in master, but I'm suggesting we use the .value trick for now. So my idea that we would then:

1. leave it in permanently for the 1.0.x branch, which doesn't have quantity_support
2. In master/1.2, re-work the example to truly use quantity_support - e.g. have it auto-convert something from days to hours and/or auto-label with "h". But while doing that, the .value trick will just make it work without requiring people to figure out why they'd really care about quantity_support.
3. in 1.1.x we could use either of the two above (but at least the next release, which we want to get out ASAP would have a working example)

whether quantity_support should be on by default, but maybe we can leave that for a separate issue?

definitely!

So if that sounds good to you, @mhvk, I think the thing to do is issue a new PR that just does the .value change in the example, which we backport to both 1.0.x and 1.1.x, and then have this rebased on that, but only get backported to 1.1.x. Then a later third PR would add "real" quantity_support()

@mhvk - actually I just went ahead and did the first step in #4657 - assuming that passes and you are OK with the plan I outlined above, you can rebase on that, excise the quantity_support() call here, and then I think this should be all set. (Although you might want to add an explicit test if you can, as the fix is no longer actually tested if you remove quantity_support() from the example...

One other consideration is whether we want to also backport the change to testing "stable" into 1.0.x. I'll just do a quick test now on my personal travis to see if it works as-is. I suspect it'll be easier to just do that directly in the v1.0.x branch rather than splitting this yet again into two PRs, though.

EDIT: looks like it will work when this is merged (the 2.6 failure is unrelated), so I'll probably go ahead and update that with the various backports: https://travis-ci.org/eteq/astropy/jobs/112999556

@eteq - the scheme is great. As you'll have seen, I've merged your PR implementing the .value option, and rebased this one after, so that it just does the update to .travis.yml. Furthermore, I opened a new PR for the fix to quantity_support in #4654

@eteq - sadly the fix you had in #4657 was incomplete (should really have noticed that...) in that it corrected the example as shown to the reader, but not the actual code producing the figure. I now added this back in here, since that way the code will actually get tested.

Oops, good catch @mhvk! In fact I did make the fix in the figure-producing code when I realized it was failing the sphinx build on my local machine... But now I realize that I forgot to actually commit that 😢.

Once the tests pass I'll go ahead and merge this, then, and backport it to 1.0.x and 1.1.x - that will at least get the full numpy version tests into those branches so we'll know if we're set.

Appveyer is a red herring, so all is good. Merging!