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

Fixed handling of aggregate extent and image size on geotiff write. #362

Merged
merged 4 commits into from Sep 23, 2019

Conversation

metasim
Copy link
Member

@metasim metasim commented Sep 22, 2019

Closes #360

Screen Shot 2019-09-22 at 4 23 18 PM

Screen Shot 2019-09-22 at 4 23 47 PM

@metasim metasim requested a review from vpipkt Sep 22, 2019
@metasim metasim added this to the 0.8.2 milestone Sep 23, 2019
Copy link
Member

@vpipkt vpipkt left a comment

Just one little nitpick in the GeoTiffDataSource.

throw new IllegalArgumentException("A destination CRS must be provided")
)

val input = df.asLayerSafely.map(layer =>
Copy link
Member

@vpipkt vpipkt Sep 23, 2019

Choose a reason for hiding this comment

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

Perhaps add an overload for TileRasterizerAggregate.collect where the type of the first arg is RasterFrameLayer that does some of this work? Feels noisy and implementation-y here.

Copy link
Member Author

@metasim metasim Sep 23, 2019

Choose a reason for hiding this comment

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

Being a layer isn't a precondition for calling this.... just needs a destination CRS, and if one isn't provided attempts to get it from the TLM if the DF is already a layer.

@@ -4,6 +4,7 @@

### 0.8.2

* Fixed handling of aggregate extent and image size on GeoTIFF writing. ([#362](https://github.com/locationtech/rasterframes/issues/362))
Copy link
Member

@vpipkt vpipkt Sep 23, 2019

Choose a reason for hiding this comment

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

Some good citizen clean up things to consider:

Suggested change
* Fixed handling of aggregate extent and image size on GeoTIFF writing. ([#362](https://github.com/locationtech/rasterframes/issues/362))
* Bumped Spark dependency to version 2.3.4. #350
* Fixed handling of aggregate extent and image size on GeoTIFF writing. ([#362](https://github.com/locationtech/rasterframes/issues/362))

vpipkt
vpipkt approved these changes Sep 23, 2019
@metasim metasim merged commit e313542 into locationtech:develop Sep 23, 2019
2 checks passed
@metasim metasim deleted the fix/360 branch Sep 23, 2019
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.

2 participants