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

update installation check notebook #24

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

Conversation

mperrin
Copy link
Collaborator

@mperrin mperrin commented Oct 16, 2020

Some straightforward updates to stsci_pancake_installation.ipynb:

  • remove now-irrelevant outdated text for Python 2, super old versions of WebbPSF, etc.
  • update target to a more sensible scene, so the output test calculation is not just noise
  • show target image as well as subtracted image for test calculation

I also add a copy of the same tests to a new file jwst_pancake/tests/test_installation.py for eventual use in CI testing. See #3 :-)

The notebook output should now look like:
Screen Shot 2020-10-16 at 1 46 21 PM

…2, update target to a more sensible scene, show target image as well as subtracted
@mperrin mperrin requested a review from juliengirard October 16, 2020 17:57
@mperrin mperrin requested a review from york-stsci October 16, 2020 18:35
@york-stsci
Copy link
Collaborator

I can clone this repository and test out the notebook, but I haven't worked on the code in a couple of years at least. How important is my review in particular as someone who isn't a developer on this project anymore, and doesn't have any time assigned to work on it.

@mperrin
Copy link
Collaborator Author

mperrin commented Oct 16, 2020

@york-stsci your review is not at all necessary in that case!

Both the GitHub auto-suggest and @lpueyo suggested I tag you, but it sounds like that's outdated info in both cases. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants