HadoopGeoTiffRDD and S3GeoTiffRDD add overloads to use file names in keys by pomadchin · Pull Request #2050 · locationtech/geotrellis

pomadchin · 2017-03-10T15:52:08Z

Fixes #1958 without a significant API change. This PR introduces a keyTransform: (URI, I) => K function to have ability to read necessary metadata from files paths, and to modify Input key.

Tests

Signed-off-by: Grigory Pomadchin <[email protected]>

lossyrob · 2017-03-12T05:46:29Z

This breaks API - I'd want to think through this in a way that won't break the API if possible... Is this the only way to accomplish this?

lossyrob · 2017-03-12T05:50:14Z

Maybe if we just create the necessary overloads of public methods to maintain the signatures that existed before, this will work.

lossyrob

Needs overloads to not break public API.

lossyrob · 2017-03-12T05:50:50Z

+    * @tparam K
+    * @return
+    */
+  def keyTransformId[K] = (_: URI, key: K) => key


Unclear that this is necessary, if the overloads are properly aligned.

pomadchin · 2017-03-12T05:51:55Z

@lossyrob sure: first approach to create smth like spark did: ~ NewHadoopGeoTiffRDD and to deprecate the old one, or I can create functions with keyTransform different and still to keep them. P.S. all public signatures are here, except old apply

P.P.S. I wrote it before comments above, didn't notice;

lossyrob · 2017-03-12T06:47:46Z

We can just overload apply to pass through the same sort of default keyTransform as the other methods.

Signed-off-by: Grigory Pomadchin <[email protected]>

pomadchin · 2017-03-13T12:15:56Z

Usage example are in tests and here.

lossyrob · 2017-03-13T15:38:33Z

Is there a good spot in docs for noting this?

pomadchin · 2017-03-13T15:51:45Z

@lossyrob eh we don't have a section for it, @fosskers can you navigate me where it would be better to write this information? Looks like it should be done as a separate PR, as it would include the whole new section with this part of API description.

fosskers · 2017-03-13T15:59:42Z

What needs to be noted, exactly? If there was a deprecation, it should be explained in a @deprecated annotation on the old, overloaded method. The note will appear at compile time for the user, so that they'll know they have to switch eventually.

I don't think deprecation notices need to go into ReadTheDocs, other than in the CHANGELOG.

echeipesh · 2017-03-13T16:12:12Z

-      rr.readWindow(reader, pixelWindow, options)
+      val (k, v) = rr.readWindow(reader, pixelWindow, options)
+
+      keyTransform(new URI(objectRequest.getKey), k) -> v


getKey will only give you the path without the s3://bucket/ prefix

echeipesh · 2017-03-13T16:13:32Z

+    * @param bucket Name of the bucket on S3 where the files are kept.
+    * @param prefix Prefix of all of the keys on S3 that are to be read in.
+    * @param keyTransform function to transform input key basing on the URI information.
+    */


keyTransform is a little overloaded with the way we use it for SFC. This should be renamed to uriToKey or something similar.

echeipesh · 2017-03-13T16:19:36Z

    * @param options  An instance of [[Options]] that contains any user defined or default settings.
    */
-  def apply[K, V](path: Path, options: Options = Options.DEFAULT)(implicit sc: SparkContext, rr: RasterReader[Options, (K, V)]): RDD[(K, V)] = {
+  def apply[I, K, V](path: Path, keyTransform: (URI, I) => K, options: Options)(implicit sc: SparkContext, rr: RasterReader[Options, (I, V)]): RDD[(K, V)] = {


Not certain if use of URI here provides opportunity to use a more general function. We should probably just use Path here.

pomadchin added the in progress label Mar 10, 2017

pomadchin changed the title ~~Collect metadata from file names~~ [WIP] Collect metadata from file names Mar 10, 2017

add keyTransform function

80ffe05

Signed-off-by: Grigory Pomadchin <[email protected]>

pomadchin force-pushed the feature/input-path-handle branch from 50f42c3 to 80ffe05 Compare March 10, 2017 16:30

pomadchin added this to the 1.1 milestone Mar 12, 2017

pomadchin mentioned this pull request Mar 12, 2017

Keeping filepath in HadoopGeoTiffRDD #1958

Closed

lossyrob modified the milestones: 1.2, 1.1 Mar 12, 2017

lossyrob suggested changes Mar 12, 2017

View reviewed changes

code cleanup according to Robs comments

ccfdb65

Signed-off-by: Grigory Pomadchin <[email protected]>

pomadchin changed the title ~~[WIP] Collect metadata from file names~~ Collect metadata from file names Mar 13, 2017

add tests to demonstrate how key transform function can be used

fb01b0b

Signed-off-by: Grigory Pomadchin <[email protected]>

pomadchin force-pushed the feature/input-path-handle branch from ece8450 to fb01b0b Compare March 13, 2017 11:26

code cleanup

c333be3

Signed-off-by: Grigory Pomadchin <[email protected]>

pomadchin removed the in progress label Mar 13, 2017

pomadchin modified the milestones: 1.2, 1.1 Mar 13, 2017

echeipesh requested changes Mar 13, 2017

View reviewed changes

lossyrob approved these changes Mar 13, 2017

View reviewed changes

Rename keyTransform to uriToKey

0e7c386

echeipesh approved these changes Mar 14, 2017

View reviewed changes

Rename keyTransform to pathToKey

004d984

echeipesh force-pushed the feature/input-path-handle branch from ec71d1f to 004d984 Compare March 14, 2017 01:44

echeipesh changed the title ~~Collect metadata from file names~~ HadoopGeoTiffRDD and S3GeoTiffRDD add overloads to use file names in keys Mar 14, 2017

echeipesh merged commit 580fa28 into locationtech:master Mar 14, 2017

Navigation Menu

Repository navigation

HadoopGeoTiffRDD and S3GeoTiffRDD add overloads to use file names in keys - #2050

HadoopGeoTiffRDD and S3GeoTiffRDD add overloads to use file names in keys#2050
echeipesh merged 6 commits into
locationtech:masterfrom
pomadchin:feature/input-path-handle

pomadchin commented Mar 10, 2017 •

edited

Loading

Uh oh!

lossyrob commented Mar 12, 2017

Uh oh!

lossyrob commented Mar 12, 2017

Uh oh!

lossyrob left a comment

Uh oh!

lossyrob Mar 12, 2017

Uh oh!

pomadchin commented Mar 12, 2017 •

edited

Loading

Uh oh!

lossyrob commented Mar 12, 2017

Uh oh!

pomadchin commented Mar 13, 2017 •

edited

Loading

Uh oh!

lossyrob commented Mar 13, 2017

Uh oh!

pomadchin commented Mar 13, 2017

Uh oh!

fosskers commented Mar 13, 2017 •

edited

Loading

Uh oh!

echeipesh Mar 13, 2017

Uh oh!

echeipesh Mar 13, 2017

Uh oh!

echeipesh Mar 13, 2017 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

pomadchin commented Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lossyrob commented Mar 12, 2017

Uh oh!

lossyrob commented Mar 12, 2017

Uh oh!

lossyrob left a comment

Choose a reason for hiding this comment

Uh oh!

lossyrob Mar 12, 2017

Choose a reason for hiding this comment

Uh oh!

pomadchin commented Mar 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lossyrob commented Mar 12, 2017

Uh oh!

pomadchin commented Mar 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lossyrob commented Mar 13, 2017

Uh oh!

pomadchin commented Mar 13, 2017

Uh oh!

fosskers commented Mar 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

echeipesh Mar 13, 2017

Choose a reason for hiding this comment

Uh oh!

echeipesh Mar 13, 2017

Choose a reason for hiding this comment

Uh oh!

echeipesh Mar 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pomadchin commented Mar 10, 2017 •

edited

Loading

pomadchin commented Mar 12, 2017 •

edited

Loading

pomadchin commented Mar 13, 2017 •

edited

Loading

fosskers commented Mar 13, 2017 •

edited

Loading

echeipesh Mar 13, 2017 •

edited

Loading