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

feat(attribute-completeness): support custom filter #848

Merged
merged 13 commits into from
Dec 5, 2024
Merged

Conversation

matthiasschaub
Copy link
Collaborator

@matthiasschaub matthiasschaub commented Nov 14, 2024

Description

Support custom attribute definition via ohsome filter query for the Attribute Completeness indicator. Now, the API endpoint attribute-completeness takes either attributeKeys as before or attributeFilter and attributeNames as parameters. The first is an ohsome filter query and the second is a list of attribute names represented by the ohsome filter query.

  • feat(attribute-completeness): support custom filter
  • test(api): factor out test for attribute completeness
  • feat(api): support custom attribute filter
  • refactor: transform model vals & use args/kwargs

Corresponding issue

Closes #840

New or changed dependencies

  • None

Checklist

  • 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 force-pushed the ac-filter branch 2 times, most recently from d417bdd to 52e6764 Compare November 14, 2024 23:25
support custom attribute filter (given as ohsome filter query) for the
attribute completeness indicator
@matthiasschaub matthiasschaub force-pushed the ac-filter branch 2 times, most recently from 7c2f24a to c2b1b28 Compare November 18, 2024 03:28
@matthiasschaub matthiasschaub marked this pull request as ready for review November 18, 2024 04:34
@matthiasschaub matthiasschaub force-pushed the ac-filter branch 2 times, most recently from daf61b7 to a37e06d Compare November 20, 2024 00:58
keep name closer to original pytest nodeid, but move approved files to
own directory "tests/approved/{module}".
support custom attribute filter (given as ohsome filter query) for API
endpoint /indicators/attribute-completeness
First transform model values from enums to dataclasses (e.g. TopicEnum
-> TopicDefinition).

Then use unpacking of arguments and keyword arguments to make function
calls simpler.
check is not needed anymore because of request validation by pydantic
models
@matthiasschaub matthiasschaub added the enhancement New feature or request label Nov 20, 2024
Copy link
Collaborator

@Gigaszi Gigaszi left a comment

Choose a reason for hiding this comment

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

looks nice. We will test it in combination with the frontend. Maybe we should add hurl tests for the custom filter

@@ -49,29 +54,54 @@ class BaseBpolys(BaseConfig):

@field_validator("bpolys")
@classmethod
def transform(cls, value) -> geojson.FeatureCollection:
def transform_bpolys(cls, value):
Copy link
Collaborator

Choose a reason for hiding this comment

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

why remove the return type here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No reason. I re-added the type hint!

result = round(self.result.value * 100, 1)
if self.attribute_title is None:
raise TypeError("Attribute names should not be None.")
else:
Copy link
Collaborator

Choose a reason for hiding this comment

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

why use names here and not title?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. I started out with attribute names and changed it to title later on. Will correct it here.

@mmerdes mmerdes requested a review from Gigaszi December 5, 2024 13:55
@mmerdes mmerdes merged commit 3204b41 into main Dec 5, 2024
2 checks passed
@mmerdes mmerdes deleted the ac-filter branch December 5, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(attribute-completeness): support custom attribute definition via ohsome filter
3 participants