Don't use stsynphot, use plain synphot. #421
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s of unrelated reference data
Codecov Report
@@ Coverage Diff @@
## develop #421 +/- ##
===========================================
- Coverage 74.27% 74.26% -0.02%
===========================================
Files 18 18
Lines 6087 6084 -3
===========================================
- Hits 4521 4518 -3
Misses 1566 1566
Continue to review full report at Codecov.
|
shanosborne
reviewed
Apr 1, 2021
shanosborne
reviewed
Apr 1, 2021
| @@ -1465,7 +1468,7 @@ def specFromSpectralType(sptype, return_list=False, catalog=None): | |||
| import os | |||
| cdbs = os.getenv('PYSYN_CDBS') | |||
| if cdbs is None: | |||
| raise EnvironmentError("Environment variable $PYSYN_CDBS must be defined for stsynphot") | |||
| raise EnvironmentError("Environment variable $PYSYN_CDBS must be defined for synphot") | |||
There was a problem hiding this comment.
Does PYSYN_CDBS also need to be defined for synphot? Or should this line stay stsynphot? I thought it was stsynphot specific, but I may be wrong
shanosborne
approved these changes
Apr 1, 2021
|
That one function - looking up for spectral type to a stellar spectrum - may be the one piece that for now actually does require stsynphot. But that's an optional piece of functionality which many users never use, so I'm OK with that. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Swap to synphot instead of pysynphot for poppy, at basically no loss in functionality we care about. This avoids needing to download many GBs of unrelated reference data.
The HST calibration spectral dataset is 4.25 GB. That's not acceptable to force as a required download on all users, given the diverse set of use cases for poppy which are in some cases completely unrelated to HST/JWST data. (e.g. optics lab technology work and cubesat optical modeling)