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

integrate ohsome filter #306

Merged
merged 165 commits into from
Dec 9, 2020
Merged

integrate ohsome filter #306

merged 165 commits into from
Dec 9, 2020

Conversation

tyrasd
Copy link
Member

@tyrasd tyrasd commented Dec 8, 2020

see #302

New or changed dependencies:

  • dropped filter library, integrated here directly

Checklist

avoids casting of result object -> good
still to improve, though: for example do not create a separate parser object for each request.
* rename variable to match code style guidelines
* reoreder and complete javadoc at-clauses
were adapted from the current version used by the ohsome-api, with minor modifications
allowed are: a-z, A-Z, digits, colon, underscore and dash.
@tyrasd tyrasd added this to the release 0.7.0 milestone Dec 8, 2020
@tyrasd tyrasd self-assigned this Dec 8, 2020
@tyrasd tyrasd changed the title WIP: integrate ohsome filter 🚧 integrate ohsome filter Dec 9, 2020
@tyrasd tyrasd force-pushed the integrate-ohsome-filter branch from ce503b0 to 54a24df Compare December 9, 2020 14:27
only the regex pattern matching version doesn't have a corresponding solution in the new filter module, so marking all but that one as deprecated
@tyrasd tyrasd changed the title 🚧 integrate ohsome filter integrate ohsome filter Dec 9, 2020
@tyrasd
Copy link
Member Author

tyrasd commented Dec 9, 2020

documentation, examples (and benchmarks?) will be updated in a separate PR

@tyrasd tyrasd requested a review from joker234 December 9, 2020 15:00
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.

Open ToDo:

CHANGELOG.md Show resolved Hide resolved
oshdb-filter/README.md Show resolved Hide resolved
oshdb-filter/README.md Show resolved Hide resolved
oshdb-filter/pom.xml Show resolved Hide resolved
@joker234
Copy link
Member

joker234 commented Dec 9, 2020

We don't want to keep the ohsome-filter Changelog, correct? Removed in c15e945

@tyrasd
Copy link
Member Author

tyrasd commented Dec 9, 2020

We don't want to keep the ohsome-filter Changelog, correct? Removed in c15e945

not really – it's still available in the original repository for reference

@joker234
Copy link
Member

joker234 commented Dec 9, 2020

General comment: The review only included all files outside of /oshdb-filter and all changes in /oshdb-filter between 633bdcb and 0f50415 (current integrate-ohsome-filter). The changes before this commit are not reviewed as they were part of the old repository already.

@tyrasd
Copy link
Member Author

tyrasd commented Dec 9, 2020

missing @deprecated? [x2]

no, because the oshdb-filter doesn't have an equivalent for the String key, Pattern valuePattern case at the moment. see https://gitlab.gistools.geog.uni-heidelberg.de/giscience/big-data/ohsome/libs/ohsome-filter/-/issues/11

@tyrasd
Copy link
Member Author

tyrasd commented Dec 9, 2020

change deprecation reference to new filter for all where(…) functions?

ui, I totally forgot these still exist. I think it's time to drop them with 0.7, but let's do that in a separate PR

@tyrasd tyrasd requested a review from joker234 December 9, 2020 16:44
@joker234 joker234 merged commit 7672641 into master Dec 9, 2020
@joker234 joker234 deleted the integrate-ohsome-filter branch December 9, 2020 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies referes to one of our dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants