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

Added documentation to the GeoTiff* files #1560

Merged
merged 2 commits into from Jul 5, 2016

Conversation

Projects
None yet
2 participants
@jbouffard
Contributor

jbouffard commented Jun 27, 2016

I added documentation to most methods and a few vals in the GeoTiff* files. There are a few more things that I want to add, but I think I'll need some things explained before I do.

@jbouffard

This comment has been minimized.

Contributor

jbouffard commented Jun 28, 2016

@lossyrob These are the methods which I'm not really getting right now: 1, 2, 3, and 4.

I know they have something to do with macros, but I don't understand it well enough to explain it in documentation.

@lossyrob

This comment has been minimized.

Member

lossyrob commented Jun 28, 2016

@MyNameIsBouf

These functions implement the interface methods of IterableTile and MappableTile[Tile], which are extended by the Tile trait (https://github.com/geotrellis/geotrellis/blob/master/raster/src/main/scala/geotrellis/raster/Tile.scala#L34)

MappleTile extends MacroMappableTile (https://github.com/geotrellis/geotrellis/blob/master/raster/src/main/scala/geotrellis/raster/MappableTile.scala#L8), and the implementation of function parameters that take more than 2 args is called via a macro (https://github.com/geotrellis/geotrellis/blob/master/raster/src/main/scala/geotrellis/raster/MappableTile.scala#L15). The IterableTile does a similar thing.

The macro methods create implementations of map and foreach that are called with "Visitor" or "Mapper" traits. These traits are new-ed up at compile time by macros like this one (https://github.com/geotrellis/geotrellis/blob/master/macros/src/main/scala/geotrellis/macros/TileMacros.scala#L34).

We don't need this crazy Macro business for function parameters with 2 or less parameters because of specialization. See how Function2 has specialized type params (https://github.com/scala/scala/blob/2.12.x/src/library/scala/Function2.scala#L29), but Function3 does not (https://github.com/scala/scala/blob/2.12.x/src/library/scala/Function3.scala#L16). Specialization is a weird and complex topic; here's an article dealing with specialization: http://axel22.github.io/2013/11/03/specialization-quirks.html

Macros are another heavy topic. Here's some docs on them: http://docs.scala-lang.org/overviews/macros/overview.html

Let me know if you have followup questions after trying to digest some of that stuff.

@jbouffard jbouffard force-pushed the jbouffard:documentation branch from e858ca4 to e6f107d Jul 5, 2016

Added documentation for the GeoTiff* files
Added documentation to the GeoTiff* files

Wording change

@jbouffard jbouffard force-pushed the jbouffard:documentation branch from e6f107d to 4556359 Jul 5, 2016

@jbouffard

This comment has been minimized.

Contributor

jbouffard commented Jul 5, 2016

@lossyrob Travis failed, but after looking at what caused the problem and double checking my code, I think there's actually nothing wrong with it. The failed test had something to do with PostgreSQL, which doesn't have anything to do with what I've done.

@@ -211,9 +230,16 @@ abstract class GeoTiffMultibandTile(
}
}
/** Converts all of the bands into a collection of Vecot[Tile] */

This comment has been minimized.

@lossyrob
/**
* Map over a MultibandTile starting at the given band.
*
* @param b0: The starting band

This comment has been minimized.

@lossyrob

lossyrob Jul 5, 2016

Member

"Starting band" reads strange here, since it's the only band in question.

trait GeoTiffSegment {
def size: Int
def getInt(i: Int): Int
def getDouble(i: Int): Double
// represents all of the bytes in the segment

This comment has been minimized.

@lossyrob

lossyrob Jul 5, 2016

Member

Docstring comments vs normal comment, needs /** */

@@ -6,6 +6,7 @@ import geotrellis.raster.io.geotiff.compression._
trait GeoTiffSegmentCollection {
type T >: Null <: GeoTiffSegment
// represents all of the segments in the geotiff

This comment has been minimized.

@lossyrob

lossyrob Jul 5, 2016

Member

Docstring comment vs normal comment

@lossyrob

This comment has been minimized.

Member

lossyrob commented Jul 5, 2016

@jbouffard make sure to mark PR's that are ready for review (and merging if it gets a +1) as not [WIP]

@jbouffard jbouffard changed the title from [WIP] Added documentation to the GeoTiff* files to Added documentation to the GeoTiff* files Jul 5, 2016

@jbouffard

This comment has been minimized.

Contributor

jbouffard commented Jul 5, 2016

@lossyrob Okay. I made the changes to both the documentation and to the title.

@lossyrob

This comment has been minimized.

Member

lossyrob commented Jul 5, 2016

👍

@lossyrob lossyrob merged commit 5b84744 into locationtech:master Jul 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@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