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

Descriptive error for post processing empty results #152

Merged
merged 11 commits into from May 21, 2020
Merged

Conversation

aspeake
Copy link
Contributor

@aspeake aspeake commented May 6, 2020

Fixes #151

Pull Request Description

Throws a more descriptive error in post-processing when no simulation results are found

Checklist

Not all may apply

  • Code changes (must work)
  • Tests exercising your feature/bug fix (check coverage report on CircleCI build -> Artifacts)
  • All other unit tests passing
  • Update validation for project config yaml file changes
  • Update documentation
  • Run a small batch run to make sure it all works (local is fine, unless an Eagle specific feature)

@aspeake aspeake requested a review from nmerket May 6, 2020 21:11
@aspeake
Copy link
Contributor Author

aspeake commented May 6, 2020

@nmerket I have added a error message when no results are found in postprocessing.py, and have checked that this error works locally, but have not written any tests for it or run on Eagle yet.

Also, I am not sure if we need a more descriptive message, I figured this issue could arise a few different ways (eg, timeout or all sims failed), so it is currently pretty vague.

@aspeake aspeake changed the title exception for empty results df Descriptive error for post processing empty results May 6, 2020
@nmerket
Copy link
Member

nmerket commented May 8, 2020

@aspeake This is a good start. I might just raise an exception with the error message. It's probably fine the way it is as far as descriptiveness. Just knowing that much will give people a clue where to start looking next.

Copy link
Member

@nmerket nmerket left a comment

Choose a reason for hiding this comment

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

This looks good to me. One suggestion below. I think it's ready. If you do too, mark it ready for review so I can merge it.

buildstockbatch/test/test_postprocessing.py Outdated Show resolved Hide resolved
Co-authored-by: Noel Merket <noel.merket@nrel.gov>
@aspeake aspeake marked this pull request as ready for review May 15, 2020 15:15
@aspeake
Copy link
Contributor Author

aspeake commented May 21, 2020

@nmerket this should be ready for review + merge

@nmerket nmerket merged commit 1e01df2 into master May 21, 2020
@nmerket nmerket deleted the issue-151 branch May 21, 2020 22:15
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.

Cryptic error message about "started_at" when there are no simulation results
2 participants