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

Fix bugs related to extent-cropping #1774

Merged
merged 2 commits into from May 30, 2023
Merged

Fix bugs related to extent-cropping #1774

merged 2 commits into from May 30, 2023

Conversation

AdeelH
Copy link
Collaborator

@AdeelH AdeelH commented Apr 13, 2023

Overview

Currently in RV, the concept of extent is overloaded to refer to the both the full extent of the data in the data source file as well as a user-specified subset of it. See #1766 for more discussion.

This PR attempts to correct this by disentangling these into extent and bbox. A RasterioSource that reads from a 100x100 GeoTIFF that is cropped to exclude 10 pixels on all sides, for example, will now have .bbox == Box(10, 10, 90, 90) and .extent == Box(0, 0, 80, 80). Moreover, the user is expected to interact with the RasterioSource using local coordinates; i.e. coordinates relative to its bbox rather than global coordinates. See "option 2" here.

Checklist

  • Added needs-backport label if PR is bug fix that applies to previous minor release
  • Ran scripts/format_code and committed any changes
  • Documentation updated if needed
  • PR has a name that won't get you publicly shamed for vagueness

Notes

N/A

Testing Instructions

See updated unit tests.

Closes #1766

@AdeelH AdeelH force-pushed the extent_fix branch 2 times, most recently from 5e6bf17 to a4f5359 Compare April 13, 2023 17:17
@AdeelH AdeelH force-pushed the extent_fix branch 2 times, most recently from 7f46adb to a553811 Compare May 30, 2023 17:25
@AdeelH AdeelH marked this pull request as ready for review May 30, 2023 17:25
@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #1774 (fd1e94e) into master (cca2580) will increase coverage by 0.02%.
The diff coverage is 85.24%.

❗ Current head fd1e94e differs from pull request most recent head 71b14fe. Consider uploading reports for the commit 71b14fe to get more accurate results

@@            Coverage Diff             @@
##           master    #1774      +/-   ##
==========================================
+ Coverage   79.37%   79.40%   +0.02%     
==========================================
  Files         186      186              
  Lines        8733     8726       -7     
==========================================
- Hits         6932     6929       -3     
+ Misses       1801     1797       -4     
Impacted Files Coverage Δ
...rvision_core/rastervision/core/data/label/utils.py 100.00% <ø> (ø)
...sion/core/data/label_source/label_source_config.py 100.00% <ø> (ø)
.../rastervision/core/data/label_store/label_store.py 100.00% <ø> (ø)
...vision/core/data/label_store/label_store_config.py 85.71% <ø> (ø)
...e/data/raster_source/multi_raster_source_config.py 68.75% <ø> (ø)
.../core/data/raster_source/rasterio_source_config.py 50.00% <ø> (ø)
...n/pytorch_backend/pytorch_semantic_segmentation.py 57.77% <ø> (ø)
...a/label_store/semantic_segmentation_label_store.py 61.79% <45.45%> (+1.68%) ⬆️
...ource/semantic_segmentation_label_source_config.py 63.15% <50.00%> (ø)
...data/label_store/object_detection_geojson_store.py 68.57% <71.42%> (+1.00%) ⬆️
... and 24 more

... and 2 files with indirect coverage changes

@AdeelH AdeelH merged commit 6cd29f8 into azavea:master May 30, 2023
1 check passed
@AdeelH AdeelH deleted the extent_fix branch June 1, 2023 13:37
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.

Inconsistent handling of extents
1 participant