Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add webbpsf and poppy to base environment #58

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Oct 3, 2022

closes #56

TODO: add test data and enable tests for both packages

@zacharyburnett zacharyburnett added the add package add a new package to the environment label Oct 3, 2022
@zacharyburnett zacharyburnett self-assigned this Oct 3, 2022
@ojustino
Copy link

ojustino commented Oct 3, 2022

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

@zacharyburnett
Copy link
Collaborator Author

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?

@ojustino
Copy link

ojustino commented Oct 3, 2022

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.

@zacharyburnett
Copy link
Collaborator Author

Thank you! That's all I need.

@zacharyburnett
Copy link
Collaborator Author

zacharyburnett commented Oct 4, 2022

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 zacharyburnett force-pushed the add_webbpsf_poppy branch 2 times, most recently from 4de0350 to 7112133 Compare October 5, 2022 16:18
@zacharyburnett
Copy link
Collaborator Author

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 zacharyburnett force-pushed the add_webbpsf_poppy branch 3 times, most recently from 297356d to d6bdc20 Compare October 12, 2022 13:55
@zacharyburnett
Copy link
Collaborator Author

zacharyburnett commented Oct 12, 2022

@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.

@mperrin
Copy link

mperrin commented Oct 17, 2022

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.

Copy link

@mperrin mperrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds hundreds of files which should not be added into this repo. Let's discuss other better ways to do this, please.

@mperrin
Copy link

mperrin commented Oct 17, 2022

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?

@zacharyburnett
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

@mperrin
Copy link

mperrin commented Oct 17, 2022

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.

@zacharyburnett zacharyburnett force-pushed the add_webbpsf_poppy branch 2 times, most recently from 26cfa2f to 920968c Compare October 18, 2022 13:58
@github-actions github-actions bot added environment changes to the base environment testing changes to automation and testing labels Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add package add a new package to the environment environment changes to the base environment testing changes to automation and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add webbpsf and poppy to environment
3 participants