desihub / specsim

Quick simulations of spectrograph response
2 stars 9 forks source link

Fix error in converting focal plane coordinates to angles #77

Closed julienguy closed 7 years ago

julienguy commented 7 years ago

This fixes an error in the conversion of focal plane coordinates to angles on the sky when computing fiber offsets.

sbailey commented 7 years ago

Odd recursion build doc test failure. Can you look into this @dkirkby or @weaverba137 ?

weaverba137 commented 7 years ago

That's not a doc test failure, that's a failure to install speclite. Is this actually reproducible?

weaverba137 commented 7 years ago

As far as the other failure goes, I don't think the bleeding-edge development version of astropy is Python 2 compatible anymore.

Since specsim is ostensibly an astropy-affiliated package, make absolutely certain that it is up-to-date with all the latest requirements.

sbailey commented 7 years ago

The speclite installation failure was repeatable for that Travis job, but it doesn't fail for most of the Travis test configurations so I'm not sure why.

It is unclear if the py=2.7 + astropy=dev combination is supposed to pass anymore.

Neither of these failures is related to the bug fix that we are trying to merge, but I'd rather not break tests on master (though it's possible that they are already broken due to astropy evolving).

@dkirkby please take a look.

dkirkby commented 7 years ago

I can look at this on Monday. Is getting this fix into master blocking anyone's immediate progress?

dkirkby commented 7 years ago

As a sanity check of the speclite install problem, I verified that I can execute the following w/o surprises:

conda create --name speclite-test pip numpy scipy astropy
source activate speclite-test
pip install speclite==0.5
source deactivate
conda remove --name speclite-test --all
dkirkby commented 7 years ago

As another sanity check, I confirmed that I can build the docs locally using python setup.py build_docs.

dkirkby commented 7 years ago

I fixed the py2.7 + astropy-dev problem. Next, I am trying some changes to speclite to fix the build_docs problem...

dkirkby commented 7 years ago

I think I have identified the problems now (see astropy/package-template#217). Once I have all the speclite tests passing, I will tag speclite 0.6 and require it for specsim tests.

dkirkby commented 7 years ago

I have disabled the build_doc test for now since I don't understand the problem but it doesn't need to block this PR. I will merge when/if the latest tests pass.