paleolimbot commented on code in PR #1023:
URL: https://github.com/apache/sedona-db/pull/1023#discussion_r3531065923
##########
docs/reference/sql/s2_coveringcellids.qmd:
##########
@@ -29,8 +35,20 @@ Returns an array of 64-bit S2 cell IDs that form a covering
of the given geograp
This is useful for spatial indexing and efficient spatial queries. The
returned cells may be at different S2 levels depending on the size and shape of
the geography.
+Optional positional arguments can control the covering:
+
+| Argument | Description | Default |
+| --- | --- | --- |
+| `min_level` | Minimum S2 cell level to return. Must be between 0 and 30. | 0
|
+| `max_level` | Maximum S2 cell level to return. Must be between 0 and 30. |
30 |
+| `max_cells` | Maximum number of cells to return. Must be greater than 0. | 8
|
Review Comment:
Instead of as a table, can you give these labels and descriptions in the
yaml itself? You can see the documentation for st_buffer which does this.
##########
python/sedonadb/tests/geography/test_geog_s2.py:
##########
@@ -92,3 +92,14 @@ def test_s2_coveringcellids(eng, geog, expected):
)
df = eng.result_to_pandas(result)
assert len(df.iloc[0, 0]) == len(expected)
+
+
[email protected]("eng", [SedonaDB])
+def test_s2_coveringcellids_parameters(eng):
+ eng = eng.create_or_skip()
+ result = eng.execute_and_collect(
+ "SELECT S2_CoveringCellIds("
+ "ST_GeogFromText('LINESTRING (0 0, 100 50)'), 0, 30, 2)"
+ )
+ df = eng.result_to_pandas(result)
+ assert len(df.iloc[0, 0]) <= 2
Review Comment:
Can you add tests for all of the overloads that are exposed here? (min level
and min + max level overloads are missing here). They can all be in the same
Python test.
--
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]
