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

Move missing data files message inside error #391

Merged
merged 4 commits into from
Jan 6, 2021

Conversation

shanosborne
Copy link
Contributor

Closes #380

Moves missing data files print out inside the IOError/EnvironmentError call so it can be caught by users if necessary

@shanosborne shanosborne requested a review from mperrin November 16, 2020 15:20
@codecov
Copy link

codecov bot commented Nov 18, 2020

Codecov Report

Merging #391 (9827765) into develop (aaca6cf) will decrease coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@             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     
Impacted Files Coverage Δ
webbpsf/utils.py 63.60% <75.00%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aaca6cf...9827765. Read the comment docs.

Copy link
Collaborator

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

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

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}"

@shanosborne shanosborne merged commit 9ed92da into spacetelescope:develop Jan 6, 2021
@shanosborne shanosborne deleted the fix-err-messages branch July 4, 2021 21:56
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.

Have a way to suppress the data files missing error message
2 participants