astropy / astroplan

Observation planning package for astronomers – maintainer @bmorris3
https://astroplan.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
199 stars 109 forks source link

Problem with numpy 1.7 and comparing units #205

Closed kvyh closed 8 years ago

kvyh commented 8 years ago

Shown at https://travis-ci.org/astropy/astroplan/jobs/149552934, the numpy 1.7 build isn't handling comparisons between TimeDeltas and time Quantities.

I am having difficulties with getting numpy 1.7 to work on my computer (when I tried from astropy.time import Time I got RuntimeError: module compiled against API version 0xa but this version of numpy is 0x7 so I can't test why this is occurring.

I also only changed the end_time used in the scheduling, compared to a version of the test that passed the checks (https://github.com/astropy/astroplan/pull/203/files#diff-0a76986689e0ed763a45883b287ee97fL41 are the changes compared to the version that passed).

kvyh commented 8 years ago

@eteq @bsipocz any idea why the TimeDelta and Quantity comparison is having issues (or why numpy is even involved in the comparison)?

wtgee commented 8 years ago

Doesn't tb.duration become an array a few lines above? So I don't think it's the TimeDelta and Quantity check that is failing, I think it is tb.duration > new_start_time - tb.start_time which is saying [...] > 100 s.

I was working on this chunk of code last night and ran into some similar problems (along with others).

kvyh commented 8 years ago

tb.duration = times_indices * time_resolution is probably what you are referencing. times_indices is an int and time_resolution is an astropy.units.Quantity so it shouldn't be an array

wtgee commented 8 years ago

Oh, got it, sorry. I did see something similar to this last night though so I'll look through what I was doing.

On Thu, Aug 4, 2016 at 7:32 AM, Karl notifications@github.com wrote:

tb.duration = times_indices * time_resolution is probably what you are referencing. times_indices is an int and time_resolution is an astropy.Quantity so it shouldn't be an array

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/astropy/astroplan/issues/205#issuecomment-237381141, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEUUNJegh44c4MBK37XF-t9uTBcZuDfks5qcQjrgaJpZM4JcGkp .

~Wilfred Tyler Gee

kvyh commented 8 years ago

Any idea is worth looking into. That's just one case where I have a lot of familiarity with the code so I can say what it's doing.

bsipocz commented 8 years ago

I don't have time now to look into the details, but it may be worth going through the numpy changelog and see what was new in 1.8. My guess is that you use something in the new code that was not available (or buggy) in 1.7.

On 3 August 2016 at 22:47, Karl notifications@github.com wrote:

Any idea is worth looking into (that's just one case where I have a lot of familiarity with the code).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/astropy/astroplan/issues/205#issuecomment-237385105, or mute the thread https://github.com/notifications/unsubscribe-auth/AGeUwmu2Us0nT4LZE-r0_GNzbDGtp9xUks5qcQxfgaJpZM4JcGkp .

kvyh commented 8 years ago

This appears to be fixed by #232, though I have no idea why.

bmorris3 commented 8 years ago

Such mystery.