-
Notifications
You must be signed in to change notification settings - Fork 63
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
Move missing data files message inside error #391
Move missing data files message inside error #391
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #391 +/- ##
===========================================
- Coverage 48.29% 48.28% -0.01%
===========================================
Files 14 14
Lines 5721 5718 -3
===========================================
- Hits 2763 2761 -2
+ Misses 2958 2957 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An old PR that I still owed you a review for. Looks good to me but here is one optional suggestion.
webbpsf/utils.py
Outdated
@@ -201,15 +201,14 @@ def get_webbpsf_data_path(data_version_min=None, return_version=False): | |||
if path_from_config == 'from_environment_variable': | |||
path = os.getenv('WEBBPSF_PATH') | |||
if path is None: | |||
sys.stderr.write(MISSING_WEBBPSF_DATA_MESSAGE) | |||
raise EnvironmentError("Environment variable $WEBBPSF_PATH is not set!") | |||
raise EnvironmentError("{}\nEnvironment variable $WEBBPSF_PATH is not set!".format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: maybe put the lengthy error message text at the end of the exception messages instead of the front? Also, now that we're only supporting recent Pythons we can do all the string formatting with f-strings, so like:
raise EnvironmentError(f"Environment variable $WEBBPSF_PATH is not set!\n{MISSING_WEBBPSF_DATA_MESSAGE}"
Closes #380
Moves missing data files print out inside the
IOError
/EnvironmentError
call so it can be caught by users if necessary