-
Notifications
You must be signed in to change notification settings - Fork 361
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
Add TileFeature Type #1429
Add TileFeature Type #1429
Conversation
42b1f43
to
2b3c42c
Compare
2b3c42c
to
a084067
Compare
c09f39c
to
00c325f
Compare
0ec6d21
to
0c4c522
Compare
0c4c522
to
cfc2edf
Compare
dd0c48b
to
dc7c7ee
Compare
60aaec9
to
c18d68c
Compare
import org.apache.spark.rdd._ | ||
import org.joda.time.DateTime | ||
|
||
trait LayerUpdateSpaceTimeTileFeatureTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FunSpec, should be named Spec
instead of Tests
. Tests
represents FunSuite
tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I named this one after the file .../spark/io/LayerUpdateSpaceTimeTests.scala
from which it was copied (same is true for .../spark/io/CoordinateSpaceTimeTileFeatureTests.scala
and .../spark/io/AllOnesTestTileFeatureTests.scala
). I can change the names of one or both of these sets of tests, if desired.
+1 after comments addressed |
All comments have been addressed with the exception of the |
Just to amplify, I am curious to know whether I should just change the names of the files that I added, or also change the name sof the files that they are based on. |
The latter choice, change all the incorrect ones. Thanks. |
Okay, will-do. |
fe2182a
to
b6fbc65
Compare
All comments prior to this one have been addressed. |
In this pull request the
TileFeature[T, D]
type is added.Points to Examine
TheTileFeature[T <: CellGrid,D: ? => BundleMethods[D]]
type takes two type parameters, but themerge
methods (in bothraster
andspark
assume thatT <: Tile
. There are no assumptions onD
other than it being a member of the bundle type class.There is a question of range and precision of the data D. Right now they are treated as integer entities. Double versions are possible as well, but since one of the supported types ofD
isSeq[Tile]
, there is a question of how (best) to handle the situation when the data are mixed integers and floats.I found that I had to add these secondary conversions from subclasses ofBundleMethods[D]
toBundleMethods[D]
in order to get the constraints onTileFeatureMergeMethods
to be satisfied. The issue seems to be that the Scala compiler does not look more than one layer deep when trying to satisfy these constraints. If there is a better way to handle this, I am open to it.Still Need To
Provide Bundle Methods forSeq[Tile]
TileFeature
sProvide Merge Methods for RDDs ofTileFeature
sMove bundle methods to their own directoryPersistenceSpec