stenv icon indicating copy to clipboard operation
stenv copied to clipboard

add `webbpsf` and `poppy` to base environment

Open zacharyburnett opened this issue 3 years ago • 12 comments

closes #56

TODO: add test data and enable tests for both packages

zacharyburnett avatar Oct 03 '22 18:10 zacharyburnett

Hi @zacharyburnett, WebbPSF's associated data files are stored in this Box folder.

ojustino avatar Oct 03 '22 18:10 ojustino

Hi @zacharyburnett, WebbPSF's associated data files are stored in this Box folder.

Thanks! However I can't access the folder, would it be possible to give me access?

zacharyburnett avatar Oct 03 '22 18:10 zacharyburnett

Right, the folder is restricted but links to its individual files are public. Do you need more access than that? These are the links to the full and minimal files for the latest WebbPSF release.

ojustino avatar Oct 03 '22 21:10 ojustino

Thank you! That's all I need.

zacharyburnett avatar Oct 04 '22 12:10 zacharyburnett

the poppy tests pass on my local machine, but fail within the context of the macos-latest image, with the following: https://github.com/spacetelescope/stenv/actions/runs/3182721627/jobs/5189366880#step:17:115

AssertionError: Pixel values different more than expected; max relative difference is 0.10739394972269882
assert 0.10739394972269882 < 0.001

zacharyburnett avatar Oct 04 '22 14:10 zacharyburnett

also, webbpsf has the following test failure:

https://github.com/spacetelescope/stenv/actions/runs/3191216044/jobs/5207680817#step:18:56

>       s = glob.glob(surdir+'/example_alldof_A1-SM_sur.xml')[0]
[56](https://github.com/spacetelescope/stenv/actions/runs/3191216044/jobs/5207680817#step:18:57)
E       IndexError: list index out of range

zacharyburnett avatar Oct 07 '22 14:10 zacharyburnett

@ojustino I apologize for the delay; I was working on infrastructure updates to make iterating tests faster for stenv. For this addition, I'm having some test failures with poppy: https://github.com/spacetelescope/stenv/actions/runs/3235256747/jobs/5299649453#step:16:116

AssertionError: Pixel values different more than expected; max relative difference is 0.1073939497226991

and also an issue with webbpsf tests: https://github.com/spacetelescope/stenv/actions/runs/3235256747/jobs/5299652960#step:16:57

# Test every DOF on A1-1 and SM and check the OTE state updated accordingly
s = glob.glob(surdir+'/example_alldof_A1-SM_sur.xml')[0]
E       IndexError: list index out of range

(I assume this is an issue with test data discovery, though I am not sure)

Are these failures important? I can ask pytest to skip these specific tests if not.

zacharyburnett avatar Oct 12 '22 14:10 zacharyburnett

Hi all - this is NOT the right way to do this. Please do not add all the WebbPSF data files into this repository! That's unnecessary and furthermore would create a lot of hassles downstream with needing to keep in sync with the actual webbpsf data distro.

mperrin avatar Oct 17 '22 14:10 mperrin

When astroconda was our supported distribution mechanism, @jhunkeler set up a webbpsf-data package within astroconda contrib which included scripts to retrieve the Zip files and install from there: https://github.com/astroconda/astroconda-contrib/tree/master/webbpsf-data

That was a great way to get the data files installed , without putting a lot of binary files inside the git repo. Maybe we can revive that approach for stenv?

mperrin avatar Oct 17 '22 15:10 mperrin

Sure thing! I currently have the data files hosted via Git LFS, but we can set up a Box redirect to directly retrieve the files. How does build.sh download the data?

zacharyburnett avatar Oct 17 '22 15:10 zacharyburnett

Oh, I see now that a redirect is set up already at https://stsci.box.com/shared/static/ftj8esrt0apzbnff8j6m5kseii2jzy9e.gz. Thank you for pointing this out, this makes it easier. I will replace the LFS additions with a download

zacharyburnett avatar Oct 17 '22 15:10 zacharyburnett

it retrieves the file from the box URL. Not sure exactly what conda itself uses for the download (curl, wget, requests, or something else). I confess I personally never understood the exact details of how this was set up, but you can take a look at the files in the linked repo above.

mperrin avatar Oct 17 '22 15:10 mperrin