feat:locations.geojson file parsing and geometry validation
#1879
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was
unlinked from
issues
Oct 8, 2024
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 8dde4c0 📊 Notices ComparisonNew Errors (2 out of 1588 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1588 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
|
davidgamez
reviewed
Oct 9, 2024
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJSONFileLoader.java
Outdated
Show resolved
Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJSONFileLoader.java
Outdated
Show resolved
Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeedMetadata.java
Outdated
Show resolved
Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJSONFileLoader.java
Outdated
Show resolved
Hide resolved
This was
linked to
issues
Oct 9, 2024
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 8412859 📊 Notices ComparisonNew Errors (2 out of 1588 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1588 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
|
|
As discussed with @tzujenchanmbd, a partial list of changes:
|
|
Not sure if it's all included in this PR, just try to clarify everything regarding There are 3-levels Perhaps there could be 3 different notice name for each: |
|
@tzujenchanmbd @emmambd Could you specify what would be the columns/description for :
Could you also confirm that the names and descriptions for the existing notices are ok? (except unsupported_location_type that will be modified) |
|
Notice name, column displayed, and description lgtm from the screenshot. |
|
@cka-y Approved notice details: unsupported_geojson_type unsupported_feature_type |
|
@emmambd done ✅ |
cka-y
changed the title
cka-y
changed the title
locations.geojson file parsing and geometry validation
Visual Justification of Differences in Acceptance TestsThe acceptance tests reveal a new For
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 9e95a69 📊 Notices ComparisonNew Errors (2 out of 1588 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1588 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory ConsumptionList of 25 datasets(memory has increased).
List of 25 datasets(memory has decreased).
List of 25 datasets(no reference available).
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 2f14dca 📊 Notices ComparisonNew Errors (2 out of 1588 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1588 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1588 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory ConsumptionList of 25 datasets(memory has increased).
List of 25 datasets(memory has decreased).
List of 25 datasets(no reference available).
|
|
Do you know why for some files git knows it's a rename (e.g. GtfsGeojsonFeaturesContainer.java to GtfsGeoJsonFeaturesContainer.java) and others not (e.g. GeojsonFileLoader.java to GeoJsonFileLoader.java). |
jcpitre
reviewed
Oct 15, 2024
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java
Show resolved
Hide resolved
cka-y
requested review from
jcpitre and
davidgamez
and removed request for
davidgamez
jcpitre
reviewed
Oct 16, 2024
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java
Show resolved
Hide resolved
jcpitre
reviewed
Oct 16, 2024
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java
Outdated
Show resolved
Hide resolved
jcpitre
reviewed
Oct 16, 2024
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java
Outdated
Show resolved
Hide resolved
…nFileLoader.java Co-authored-by: jcpitre <106176106+jcpitre@users.noreply.github.com>
…nFileLoader.java Co-authored-by: jcpitre <106176106+jcpitre@users.noreply.github.com>
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit ac8e8d2 📊 Notices ComparisonNew Errors (2 out of 1602 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1602 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
jcpitre
reviewed
Oct 16, 2024
core/src/main/java/org/mobilitydata/gtfsvalidator/notice/InvalidGeometryNotice.java
Outdated
Show resolved
Hide resolved
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit ba9ab41 📊 Notices ComparisonNew Errors (2 out of 1602 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1602 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1602 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Part of #1850
This PR introduces enhancements for validating GeoJSON files. The most notable changes include the addition of new validation notice classes, the implementation of a
GeoJsonGeometryValidatorclass, and the renaming of classes to align with a consistent naming convention.Validation Notices:
InvalidGeometryNotice- Triggered when a GeoJSON file contains invalid geometry that does not adhere to the specified structure.MissingRequiredElementNotice- Raised when elements required in the GeoJSON format are absent.UnsupportedGeometryTypeNotice- Issued for geometry types that are not currently supported by our validator (supporting onlyPolygonandMultiPolygon).Dependency Updates:
org.locationtech.jts:jts-core:1.20.0to manage geometric operations in compliance with the GeoJSON standards. The update is reflected inmain/build.gradle.Geometry Validation
isValidfunction from thelocationtech.jtslibrary to verify that polygons conform to these criteria, as outlined by the OGC SFS specification, which is an implementation of the OpenGIS standards for geographic information systems. Reference documentation.Expected Behavior:
To verify these changes, you can use the provided test feed example. Below is a sample validation result, showing expected error notices:
Other notices covered in this PR:
typeExample of problem location from mdb-2052 feature
area_915(self intersection in the red square):Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything