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

Simplify and generalize geom processing #711

merged 5 commits into from Mar 19, 2019

Simplify and generalize geom processing #711

merged 5 commits into from Mar 19, 2019


Copy link

@lewfish lewfish commented Mar 7, 2019


Previously, the GeoJSON/geom processing code in RV was duplicated in many different places and did not handle all the geom types in the GeoJSON standard. This PR consolidates this processing into the VectorSource which now handles all geom types. The get_geojson method now returns "normalized GeoJSON" by default which has:

  • MultiGeoms and GeomCollections are split apart into single geoms that share the same class_id
  • Pixel-based coordinates
  • Non-polygons are converted to polygons using configurable buffering
  • If the buffer value for a class is set to None, then the geom isn't buffered. But this will raise an exception in any of the currently available LabelSourcessince they expect polygons. This functionality is there in case we need it in the future, perhaps for a Task that can directly predict points or lines.

The buffers for a VectorSource can now be configured like:

        b = GeoJSONVectorSourceConfigBuilder() \
            .with_uri(self.uri) \
            .with_buffers(line_bufs={1: 2.0}, point_bufs={2: 5.0}) \


Aside from being tested with unit and integration tests I ran the Vegas example for all three tasks and checked the debug chips. Also tested backward compatibility of prediction packages using an older prediction package.

Closes #522
Associated with azavea/raster-vision-examples#46

@lewfish lewfish added the review label Mar 7, 2019
Not actually used anywhere
@lewfish lewfish force-pushed the lf/geoms branch 2 times, most recently from c9425d2 to 72ab66d Mar 8, 2019
@lewfish lewfish force-pushed the lf/geoms branch from e9bd323 to 8e8b8ca Mar 18, 2019
lewfish added 4 commits Mar 18, 2019
Move splitting and CRS transform functionality into VectorSource to avoid duplication
and simplify other parts of the system. Also handle all geom types in a consistent way.
* Remove line_buf from RasterizedSource since it's an option to VectorSource now
* Remove a lot of geom processing code from utils, LabelSources, and LabelStores since it
now happens in VectorSource. These classes now expected "normalized" GeoJSON which
has already been cleaned, split, has a class_id, and is converted to pixel coords.
@lewfish lewfish force-pushed the lf/geoms branch from 8e8b8ca to 7793f6a Mar 18, 2019
Copy link

@codecov codecov bot commented Mar 18, 2019

Codecov Report

Merging #711 into develop will increase coverage by 0.51%.
The diff coverage is 97.08%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #711      +/-   ##
+ Coverage    71.66%   72.17%   +0.51%     
  Files          171      170       -1     
  Lines         8283     8281       -2     
+ Hits          5936     5977      +41     
+ Misses        2347     2304      -43
Impacted Files Coverage Δ
rastervision/data/ 0% <ø> (-61.37%) ⬇️
.../vector_source/ 90.56% <ø> (ø) ⬆️
...stervision/data/crs_transformer/ 64.28% <ø> (-2.39%) ⬇️
rastervision/data/label_source/ 81.81% <ø> (-8.3%) ⬇️
rastervision/data/label_store/ 93.75% <ø> (-1.49%) ⬇️
...a/label_store/ 95.65% <100%> (+0.41%) ⬆️
...ion/data/raster_source/ 77.77% <100%> (+0.56%) ⬆️
...a/label_source/ 95.65% <100%> (-1.23%) ⬇️
...stervision/data/raster_source/ 94.11% <100%> (+0.08%) ⬆️
rastervision/data/ 75.94% <100%> (-0.16%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90f4c5d...7793f6a. Read the comment docs.

@lewfish lewfish changed the title WIP: Simplify and generalize geom processing Simplify and generalize geom processing Mar 18, 2019
@lewfish lewfish merged commit 1c869ce into develop Mar 19, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/push The Travis CI build failed
continuous-integration/travis-ci/pr The Travis CI build passed
@lewfish lewfish deleted the lf/geoms branch Mar 19, 2019
@lewfish lewfish removed the review label Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant