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

Cropped Windowed GeoTiff Reading #1559

Merged
merged 8 commits into from
Sep 23, 2016
Merged

Cropped Windowed GeoTiff Reading #1559

merged 8 commits into from
Sep 23, 2016

Conversation

jbouffard
Copy link

@jbouffard jbouffard commented Jun 27, 2016 •
edited
Loading

This pull request adds the following:

  1. A new implementation of CompressedBytes called SegmentBytes that allows for reading in the GeoTiff's segments into either a ByteBuffer or an Array[Array[Byte]].
  2. GeoTiffs can now be read in via a stream.
  3. GeoTiffs now have their own crop method which can be used in conjunction with the stream reading.
  4. Reading in a windowed GeoTiff given an Extent.

lossyrob
lossyrob reviewed Jun 27, 2016
View reviewed changes
raster/src/main/scala/geotrellis/raster/io/geotiff/CompressedBytesArray.scala
import monocle.syntax.apply._
import spire.syntax.cfor._

class CompressedBytesArray(compressedBytes: Array[Array[Byte]]) extends CompressedBytes {
Copy link
Member

Choose a reason for hiding this comment

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

Naming convention goes from left being the most specific, right being the most general.
Not everything follows this convention in the base library, but we should follow it consistently in this library (and I think we generally do with a couple of exceptions perhaps).

ArrayCompressedBytes
ByteBufferCompressedBytes

would fit better.

Also perhaps a different name would be more descriptive.

SegmentBytes
ArraySegmentBytes
BufferSegmentBytes

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, those names were just place holders until I could think of something better. I like the names you proposed, and will rename the files and classes with their respective new names.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry about that, I had begun finishing up the unit tests, but I felt that I was late to make a PR so I just added my code without the tests. I'll be adding the tests soon.

Copy link
Member

Choose a reason for hiding this comment

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

No worries, just figured I'd mention it explicitly, I know this is a WIP PR :)

@lossyrob
Copy link
Member

Should have unit tests around the implementations.

@jbouffard jbouffard force-pushed the windowedgeotiff branch 2 times, most recently from 87e8c5d to a7becd1 Compare June 30, 2016 20:26
@echeipesh echeipesh changed the title [WIP] Added CompressedBytes abstraction [WIP] Windowed GeoTiff Reading Aug 22, 2016
@jbouffard
Added CompressedBytes abstraction
42b3c37 
Renamed the CompressedBytes abstractions

Renamed for real

Began writing tests

Added unit tests for ArraySegmentBytes

Made some changes to ArraySegmentBytes

Added BufferSegmentCountsTesting

Made changes to BufferSegmentBytes

Implemented SegmentBytes in the GeoTiff directory

Added map to SegmentBytes and made it work with Writer

Modified Boilerplate

Changed some of the testing

Added some more testing

Made chnages

Refactored SegmentBytes

Added the ability to read in windowed GeoTiffs to the implementations of GeoTiff

Refactored toArrayTile

Further edits to toArrayTile

Corrected toArrayTile

Made modifications

Implemented the WindowedGeoTiff class in the code

Refactored some more

Added most of the new Mutables

Added windowed reading

aslfkjsd

Reimplemented the modified mutables

Yet even more refactoring

Improved testing

Added documentation for most things

Removed CompressedBytes* files

Renamed test

Removed BufferSegmentBytes

Improved testing

made more changes to the tile files.

Modified implementation

Made more changes to the tests

Changed how rows and cols are found
@jbouffard
Implemented the cropped reading for GeoTiffs
7cbbd2f 
Made change to Float64

Improved testing to meet new implementation

Bit complete

reformatted tests

Added a note on Spec

Made a few formatting changes

Made changes to sbt for local publishing

Made improvements to Windowed Readers speed

fixed build.sbt

Made striped reading faster for all

made some improvements to the reader

Made the test a little better

Corrected error with the tiled reader7

Updated tests

Solved the issue for real this time

working on bug fixes
@jbouffard
Renamed the functions and changed their implementation
f34ffdb 
made changes to the striped reader

Tiles actually work now.

They work for real now

Implementation changed

Implementation improve and added to all types

Cleaned up CroppedGeoTiff

Cleaned up code and made a few more changes
@jbouffard
Reading in GeoTiffs is now done through their crop method
ca8fe1e 
Updates tests

Removed old Spec Name
@jbouffard
Improved speed for some bandtypes
2b82548 
Tile reading works without checking now.

Changed Int16 and UInt16

Changed formatting

Made a few more changes

Fixed Tile Reading issue

Implemented the correct API

Changed how the reading is done in single and mulitband files

Made changes to the test

Made fromInputStream implicit

made some changes to how the buffer is read

Made the testing a little better
@jbouffard
Cleaned up code and changed some of the implementation
14e791f
@jbouffard jbouffard force-pushed the windowedgeotiff branch 2 times, most recently from 1e4e407 to aeaf095 Compare August 29, 2016 14:15
@jbouffard
Cleaned up the code further
53c3e38 
Removed S3 stuff for good

Edited code some more

Improved SegmentBytesSpec

Edited CroppedWindowedGeoTiffSpec

Addd removed file
@jbouffard jbouffard changed the title [WIP] Windowed GeoTiff Reading Cropped Windowed GeoTiff Reading Aug 29, 2016
@lossyrob
Copy link
Member

@jbouffard can you add a description of the feature in the PR description? Something that if people were to find this PR after the fact, they could understand what the feature being merged in is all about.

@jbouffard
Copy link
Author

@lossyrob Yeah, I just added one.

This was referenced Aug 29, 2016
Closed
Merged
@jbouffard
Copy link
Author

@lossyrob I looked at where the checked failed, and I don't think it had to do with my code. This is the error message that caused it to fail:

The future returned an exception of type: java.sql.BatchUpdateException, with message: Batch entry 0 insert into "cities" ("name","geom") values ('Allentown,PA',?) was aborted. Call getNextException to see the cause.. (PostgisSpec.scala:112)

@jbouffard jbouffard mentioned this pull request Aug 31, 2016
Merged
3 tasks
@lossyrob
Copy link
Member

💯

@lossyrob lossyrob merged commit 6bd3052 into locationtech:master Sep 23, 2016
@fosskers fosskers mentioned this pull request Sep 27, 2016
Closed
@lossyrob lossyrob added this to the 1.0 milestone Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lossyrob lossyrob lossyrob approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jbouffard @lossyrob