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

Check for invalid indicator layer combination during initialization of indicator objects #28

Merged
merged 1 commit into from
May 31, 2021

Conversation

matthiasschaub
Copy link
Collaborator

@matthiasschaub matthiasschaub commented May 19, 2021

Corresponding issue

Closes #24

Checklist

  • Squash commits
  • I have updated my branch to main (e.g. through git rebase main)
  • My code follows the style guide and was checked with pre-commit before committing
  • I have commented my code
  • I have added sufficient unit and integration tests
  • I have updated the CHANGELOG.md

@matthiasschaub matthiasschaub self-assigned this May 19, 2021
@matthiasschaub matthiasschaub added the bug Something isn't working label May 19, 2021
@matthiasschaub matthiasschaub changed the title 🚧 Ceck for invalid indicator layer combination during initialization of indicator objects 🚧 Check for invalid indicator layer combination during initialization of indicator objects May 19, 2021
@matthiasschaub matthiasschaub force-pushed the invalid-indicator-layer-combination branch from 0178144 to 3898ae8 Compare May 19, 2021 15:05
@matthiasschaub matthiasschaub changed the title 🚧 Check for invalid indicator layer combination during initialization of indicator objects Check for invalid indicator layer combination during initialization of indicator objects May 19, 2021
@joker234 joker234 added this to the Release 0.4.0 milestone May 20, 2021
Copy link
Member

@joker234 joker234 left a comment

Choose a reason for hiding this comment

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

Some minor suggestions. Thx for implementing this check.

In general it would be nice if the (API) user would get more feedback about the actual exceptions. Most of the time they get Internal Server Error. See also #29 (comment)

CHANGELOG.md Outdated Show resolved Hide resolved
workers/tests/unittests/test_custom_exceptions.py Outdated Show resolved Hide resolved
workers/ohsome_quality_analyst/utils/exceptions.py Outdated Show resolved Hide resolved
@matthiasschaub matthiasschaub force-pushed the invalid-indicator-layer-combination branch from d8c16bb to a2628e2 Compare May 25, 2021 09:39
@matthiasschaub matthiasschaub force-pushed the invalid-indicator-layer-combination branch from bdfb994 to 9424243 Compare May 26, 2021 13:18
joker234
joker234 previously approved these changes May 27, 2021
Check for invalid indicator layer combination during initialization of indicator objects.

Improve unittests of custom errors

Co-authored-by: Johannes Visintini <johannes.visintini@heigit.org>

Recreate VCR cassette

Improve custom exception message

Make unittest for custom excepetion more clear

Add test for layer with not second filter

Update changelog

Use built-in ValueError instead of custom Error
@matthiasschaub
Copy link
Collaborator Author

@joker234 since I rebased (squashed) the commits your approval is not longer sufficient for GitHub to let me merge the PR. I did not make any changes to the code. Could you again approve the PR?

@matthiasschaub matthiasschaub merged commit e8d1019 into main May 31, 2021
@matthiasschaub matthiasschaub deleted the invalid-indicator-layer-combination branch May 31, 2021 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Find and fix test causing a 400 error from ohsome API
2 participants