skip test also on windows not only appveyor - #4565

I think the failure is unrelated to this PR but I'm not sure.

Hmm... It looks like this being appveyor-only traces back to @Cadair in b2f90ae - @Cadair, can you clarify if this is an appveyor-specific issue vs. windows always or windows sometimes? (@embray might also know)

It looks like there's a few other cases where it's appveyor-specific. @MSeifert04, do all the other tests pass for you, and only this one fails in windows?

Agree that the failure looks unrelated. I restarted the build to see if it's transient

@eteq - It's the only thrown failure on python 2.7.

With python 3.5 there are 15 other failures, but since all of these were when assert len(catched_warnings) == some_number raised an AssertionError it is very likely because of #4342.

I'm using Windows 10 (MSVC 14), Numpy 1.10.1 with astropy 1.1.1

Hmm, ok. I'm not well versed in the windows peculiarities... But do you have a sense of why this is the case? I.e., is this specific test doing something that changed recently in Win 10 or something? I just want to make sure it's not something we should actually fix, as opposed to just skipping the test.

@eteq - I think it's an overflow issue but I don't know where it happens, I don't have too much insight concerning io.fits. It's a parametrized test and only one is failing: utype = 'u8', compressed = False. I modified the test slightly so the assertion message is a bit more detailed:

AssertionError: 
Arrays are not equal

(mismatch 100.0%)
 x: array([9223372036854775805, 9223372036854775806, 9223372036854775807,
       9223372036854775808, 9223372036854775809, 9223372036854775810,
       9223372036854775811], dtype=uint64)
 y: array([18446744073709551613, 18446744073709551614, 18446744073709551615,
                          0,                    1,                    2,
                          3], dtype=uint64)

I guess something is going wrong during the hdu.scale call in https://github.com/astropy/astropy/blob/master/astropy/io/fits/tests/test_uint.py#L36 because a breakpoint (line 37) gives me:

hdu.data
array([-3 -2 -1  0  1  2  3], dtype=int64)

while for utype = 'u4', compressed = False it is:

hdu.data
array([ 2147483645  2147483646  2147483647 -2147483648 -2147483647 -2147483646
 -2147483645], dtype=int32)

I think fixing it would be great but is beyond my capabilities.

I couldn't let this issue go. I think I might have it. It passes locally and I hope it passes AppVeyor as well.

Fascinating, it's a bit odd that I've never noticed windows creates np.int arrays as np.int32 instead of np.int64 and that this causes the failure here... But it seems like it's passing on AppVeyor as well.

@MSeifert04 - wow, thanks for the investigation. It's super great to see when a PR gets rid of skipifs and/or xfails :)

Yes, but it feels like a hack since hdu.scale makes in-place array operations here and here before converting the dtype. That might be problemativ since scale and zero-point are applied while keeping the dtype (this caused the error in the first place).

Does anyone know if that is intentional? I'm very inexperienced with the interior of fits files.

This isn't a Windows thing per se, I don't think. It's a 32-bit Python thing.

Does anyone know if that is intentional? I'm very inexperienced with the interior of fits files.

I bet @embray knows?

@embray I think I'm using Anaconda 64bit on Windows 64-bit and python also says it 64bit. So it's probably a numpy 64-bit thing.

Also if you have some time, I mentioned in #4565 (comment) that the order of operations during scaling might be problematic. Could you have a look?

IIRC the FITS standard doesn't say much about whether this is correct or incorrect, and existing FITS software just wraps around for large BZERO. This has never been a problem that I can think of.

ok, thank you!

Alright, so I will make the command decision to merge this now as it gets more passing and testing. But feel free to try to re-order things to make the conversion earlier if you think that makes sense, @MSeifert04 - if the standard doesn't say we are free to do whatever we think is the most sensible, I suppose.

@eteq - I just noticed I forgot to delete the import platform again. Should I open another PR or can you manually delete it?

I also think I'll let the conversion as is. Noone has been troubled so far and currently increasing the dtype (int->float) is problemativ. If I changed the order the problem would occur while decreasing the dtype (float->int).