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.

Sign up for GitHub

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

Jump to bottom

ENH: Add transformer_options to WarpedVRT #2195

Merged
merged 2 commits into from
Jun 18, 2021
Merged

ENH: Add transformer_options to WarpedVRT #2195

merged 2 commits into from
Jun 18, 2021

Conversation

snowman2
Copy link
Member

@snowman2 snowman2 commented Jun 4, 2021

Closes #2194

@scottyhq scottyhq mentioned this pull request Jun 9, 2021
Closed
sgillies
sgillies reviewed Jun 11, 2021
View reviewed changes
rasterio/_warp.pyx Outdated
@@ -723,9 +769,11 @@ cdef class WarpedVRTReaderBase(DatasetReaderBase):
dst_width=None, width=None, dst_height=None, height=None,
src_transform=None, dst_transform=None, transform=None,
init_dest_nodata=True, src_alpha=0, add_alpha=False,
warp_mem_limit=0, dtype=None, **warp_extras):
warp_mem_limit=0, dtype=None, transformer_options=None, **warp_extras):
Copy link
Member

Choose a reason for hiding this comment

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

@snowman2 I'd like to see the transformer options folded into warp_extras as we're doing with _reproject: https://github.com/mapbox/rasterio/blob/master/rasterio/_warp.pyx#L479.

Copy link
Member Author

Choose a reason for hiding this comment

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

That definitely simplified things and brings consistency with WarpedVRT and _reproject. The only downside is that you are not warned when the options are ignored with earlier versions of GDAL.

sgillies
sgillies reviewed Jun 11, 2021
View reviewed changes
tests/test_warpedvrt.py Outdated Show resolved Hide resolved
sgillies
sgillies requested changes Jun 11, 2021
View reviewed changes
Copy link
Member

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

@snowman2 thank you! I'm looking forward to 1.3.0 and even have it in my team's planning doc for the second half of the year.

Just one change I think we should make in the interest of keep usage simple.

@sgillies sgillies self-assigned this Jun 11, 2021
@sgillies sgillies modified the milestones: 1.2.5, 1.3.0 Jun 11, 2021
@snowman2 snowman2 force-pushed the autovrt branch 2 times, most recently from cd12820 to 5029543 Compare June 12, 2021 00:57
@snowman2 snowman2 mentioned this pull request Jun 13, 2021
Merged
sgillies
sgillies approved these changes Jun 18, 2021
View reviewed changes
Copy link
Member

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

Thanks @snowman2 !

@sgillies sgillies merged commit cb4cfa9 into rasterio:master Jun 18, 2021
@snowman2 snowman2 deleted the autovrt branch June 18, 2021 15:19
@snowman2
Copy link
Member Author

Thanks for the review @sgillies 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgillies sgillies sgillies approved these changes

Assignees

@sgillies sgillies

Labels
enhancement
Projects
None yet
Milestone
1.3.0
Development

Successfully merging this pull request may close these issues.

ENH: Use GDALAutoCreateWarpedVRTEx with GDAL 3.2+
2 participants
@snowman2 @sgillies