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

Do not filter locations which do not have coordinates #461

Merged
merged 7 commits into from
Jan 21, 2022

Conversation

steffenkleinle
Copy link
Member

@steffenkleinle steffenkleinle commented Jan 18, 2022

Fixes #443

This currently has no effect since there is only one store with no coordinates (and all other required attributes set):

'Kunstverein Würzburg e.V. ("Arte Noah"), Würzburg' was filtered out because longitude or latitude are null

It is not possible to get correct coordinates here since the street and plz are those of a Postfach:

<name><![CDATA[Kunstverein Würzburg e.V. (\"Arte Noah\")]]></name>
<strasse><![CDATA[Postfach]]></strasse>
<hausnummer><![CDATA[110937]]></hausnummer>
<plz><![CDATA[97035]]></plz>

So the store is only filtered out in a different place. I think I'd merge this anyway as this may change over time and it does not hurt. If no coordinates are found we just filter it out later on.

@steffenkleinle steffenkleinle changed the title No coordinate filtering Do not filter locations which do not have coordinates Jan 18, 2022
Copy link
Member

@michael-markl michael-markl left a comment

Choose a reason for hiding this comment

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

Maybe it makes sense, to have a PreSanitizeFilter and a PostSanitizeFilter Pipeline steps for clarification 🤔

@@ -1,59 +0,0 @@
package app.ehrenamtskarte.backend.stores.importer
Copy link
Member Author

@steffenkleinle steffenkleinle Jan 20, 2022

Choose a reason for hiding this comment

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

Moved to PreSanitizeFilter (previously Filter)

@@ -1,21 +0,0 @@
package app.ehrenamtskarte.backend.stores.importer.steps
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to PreSanitizeFilter

@steffenkleinle
Copy link
Member Author

Maybe it makes sense, to have a PreSanitizeFilter and a PostSanitizeFilter Pipeline steps for clarification thinking

Done

@steffenkleinle steffenkleinle merged commit 8f69659 into main Jan 21, 2022
@steffenkleinle steffenkleinle deleted the 443-no-coordinate-filtering branch January 21, 2022 08:57
@michael-markl
Copy link
Member

I had to restart the CI from a failed state...

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.

Do not filter locations which do not have coordinates
3 participants