james-willis commented on code in PR #1778:
URL: https://github.com/apache/sedona/pull/1778#discussion_r1937834580
##########
spark/common/src/main/java/org/apache/sedona/core/spatialRDD/SpatialRDD.java:
##########
@@ -159,6 +159,32 @@ public boolean spatialPartitioning(GridType gridType)
throws Exception {
return true;
}
+ public boolean spatialParitioningWithoutDuplicates(GridType gridType) throws
Exception {
Review Comment:
is withoutDuplicates better as its own set of methods or as a bool flag? No
strong opinion but something to consider.
##########
spark/common/src/main/java/org/apache/sedona/core/spatialRDD/SpatialRDD.java:
##########
@@ -278,7 +304,7 @@ public void spatialPartitioning(SpatialPartitioner
partitioner) {
/** @deprecated Use spatialPartitioning(SpatialPartitioner partitioner) */
public boolean spatialPartitioning(final List<Envelope> otherGrids) throws
Exception {
- this.partitioner = new FlatGridPartitioner(otherGrids);
+ this.partitioner = new IndexedGridPartitioner(otherGrids);
Review Comment:
if we think this is important should we un-deprecate it? I have a similar
pattern in isochrones, but I create the IndexedGridParitioner myself because
this API is deprecated.
##########
spark/common/src/main/java/org/apache/sedona/core/spatialRDD/SpatialRDD.java:
##########
@@ -278,7 +304,7 @@ public void spatialPartitioning(SpatialPartitioner
partitioner) {
/** @deprecated Use spatialPartitioning(SpatialPartitioner partitioner) */
public boolean spatialPartitioning(final List<Envelope> otherGrids) throws
Exception {
- this.partitioner = new FlatGridPartitioner(otherGrids);
+ this.partitioner = new IndexedGridPartitioner(otherGrids);
Review Comment:
Is it right to include the overflow partition here? perhaps this is why the
API is deprecated, so you can configure the spatial partitioner on its own.
##########
spark/common/src/main/java/org/apache/sedona/core/spatialRDD/SpatialRDD.java:
##########
@@ -159,6 +159,32 @@ public boolean spatialPartitioning(GridType gridType)
throws Exception {
return true;
}
+ public boolean spatialParitioningWithoutDuplicates(GridType gridType) throws
Exception {
+ int numPartitions = this.rawSpatialRDD.rdd().partitions().length;
+ spatialPartitioningWithoutDuplicates(gridType, numPartitions);
+ return true;
+ }
+
+ public void spatialPartitioningWithoutDuplicates(GridType gridType, int
numPartitions)
+ throws Exception {
+ calc_partitioner(gridType, numPartitions);
+ partitioner = new GenericUniquePartitioner(partitioner);
+ this.spatialPartitionedRDD = partition(partitioner);
+ }
+
+ public void spatialPartitioningWithoutDuplicates(SpatialPartitioner
partitioner) {
+ partitioner = new GenericUniquePartitioner(partitioner);
+ this.spatialPartitionedRDD = partition(partitioner);
+ }
+
+ /** @deprecated Use spatialPartitioningWithoutDuplicates(SpatialPartitioner
partitioner) */
+ public boolean spatialPartitioningWithoutDuplicates(final List<Envelope>
otherGrids)
Review Comment:
why write a new deprecated API?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
