Skip to content

Improve impact calc error messages #691

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.

Sign up for GitHub

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

Jump to bottom
Merged
leonie-villiger merged 6 commits into from
Apr 17, 2023
Merged

Improve impact calc error messages #691

leonie-villiger merged 6 commits into from
Apr 17, 2023

Conversation

@leonie-villiger
Copy link
Collaborator

@leonie-villiger leonie-villiger commented Apr 5, 2023 •
edited
Loading

Changes proposed in this PR:
ImpactCalc.impact(exposures, impfset, hazard) checks that

  • exposures refers to impact functions for the given hazard type
  • impfset contains impact functions for the given hazard type.
    In case of mismatching hazard types, a comprehensible error message is produced. Currently, these cases result in the cryptic error message AttributeError: 'list' has no attribute 'calc_mdr'

This PR fixes #669

PR Author Checklist

PR Reviewer Checklist

chahank
chahank reviewed Apr 6, 2023
View reviewed changes
chahank
chahank reviewed Apr 6, 2023
View reviewed changes
chahank
chahank reviewed Apr 6, 2023
View reviewed changes
chahank
chahank reviewed Apr 6, 2023
View reviewed changes
@chahank
Copy link
Member

chahank commented Apr 6, 2023

Great solution to issue #669 ! Thanks! Just a few suggestion to make the test more efficient and clearer.

@ChrisFairless
Copy link
Collaborator

ChrisFairless commented Apr 14, 2023

I ran into this error today. An hour later I'm here and very pleased to see this fix ✨

@leonie-villiger
Copy link
Collaborator Author

leonie-villiger commented Apr 17, 2023

Thank you for the review! I think I addressed all your comments. Please merge the branch If you agree.

@leonie-villiger leonie-villiger requested a review from chahank April 17, 2023 06:27
chahank
chahank reviewed Apr 17, 2023
View reviewed changes
chahank
chahank reviewed Apr 17, 2023
View reviewed changes
chahank
chahank reviewed Apr 17, 2023
View reviewed changes
chahank
chahank reviewed Apr 17, 2023
View reviewed changes
@chahank
Copy link
Member

chahank commented Apr 17, 2023

Great, thanks! Ready to merge imho.

@chahank
Copy link
Member

chahank commented Apr 17, 2023

Welcome to the authors list!

@leonie-villiger leonie-villiger merged commit c7bcf32 into develop Apr 17, 2023
@leonie-villiger leonie-villiger mentioned this pull request Apr 17, 2023
Closed
@leonie-villiger leonie-villiger deleted the feature/improve_impact_calc_error_messages branch April 20, 2023 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chahank chahank chahank left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@leonie-villiger @chahank @ChrisFairless