Conversation
Reviewer's GuideAdjusts geo-conversion behavior to always prefer WKB with better error handling, modernizes logging and translations, reworks test model registry setup/teardown for stress tests, tightens a geo search test, and performs minor documentation cleanups. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 8 issues, and left some high level feedback:
- Importing
GEOSExceptionat module level will raise anImportErrorwhen Shapely is not installed and bypass the existingtry/except ImportErrorfallback; consider moving this import into the sametryblock as the other Shapely imports or handling its absence explicitly. - In
value_to_shape, unconditionally forcinguse_wkb = Truechanges the semantics of the function and ignores the caller-provided argument; if this is only needed for specific cases, consider adjusting the call sites or adding a separate code path rather than overriding the parameter. - The test setup now uses internal registry methods like
cls.registry._setup_models__, which appears to be a private or possibly misspelled API; it would be safer to rely on the recommended test utilities or the correct public registry methods to reduce fragility across versions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Importing `GEOSException` at module level will raise an `ImportError` when Shapely is not installed and bypass the existing `try/except ImportError` fallback; consider moving this import into the same `try` block as the other Shapely imports or handling its absence explicitly.
- In `value_to_shape`, unconditionally forcing `use_wkb = True` changes the semantics of the function and ignores the caller-provided argument; if this is only needed for specific cases, consider adjusting the call sites or adding a separate code path rather than overriding the parameter.
- The test setup now uses internal registry methods like `cls.registry._setup_models__`, which appears to be a private or possibly misspelled API; it would be safer to rely on the recommended test utilities or the correct public registry methods to reduce fragility across versions.
## Individual Comments
### Comment 1
<location> `base_geoengine/geo_convertion_helper.py:23-24` </location>
<code_context>
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object"""
+
+ # always use_wkb for now
+ use_wkb = True
+
if not value:
</code_context>
<issue_to_address>
**issue:** Overriding `use_wkb` unconditionally makes the function parameter misleading and may mask configuration issues.
Forcing `use_wkb = True` here makes the argument meaningless and ignores caller intent. If this is intentional but temporary, either remove the parameter and update callers, or hide this behind a dedicated configuration flag. At minimum, add a clear TODO explaining why this override exists and under what conditions it should be reverted to avoid this silent behavior change becoming permanent.
</issue_to_address>
### Comment 2
<location> `base_geoengine/geo_convertion_helper.py:38-40` </location>
<code_context>
- return wkt.loads(value)
+ res = wkb.loads(value, hex=True)
+ return res
+ except GEOSException as e:
+ _logger.warning("GEOSException:")
+ _logger.warning(e)
except Exception as e:
- logger.warning(_("Failed to parse WKT: %s", e)) # pylint: disable=prefer-env-translation
</code_context>
<issue_to_address>
**suggestion:** Exception logging for GEOS/WKB failures could include more context to aid debugging.
Right now the GEOSException handler logs only a label and the exception. Please log some minimal context about the input (e.g. type and/or length) and pass `exc_info=True` so we get the traceback, for example:
```python
_logger.warning(
"GEOSException while loading WKB (len=%s)",
len(value),
exc_info=True,
)
```
The same improvement would help in the generic WKB conversion failure case as well.
</issue_to_address>
### Comment 3
<location> `base_geoengine/static/description/index.html:434` </location>
<code_context>
<p>When you will install the module this two additional libs will be
-installed.</p>
-<p>For a complete documentation please refer to the <a class="reference external" href="http://oca.github.io/geospatial/index.html">public
+installed. For a complete documentation please refer to the <a class="reference external" href="http://oca.github.io/geospatial/index.html">public
documenation</a></p>
</div>
</code_context>
<issue_to_address>
**nitpick (typo):** Minor spelling issue in the updated sentence.
The link text currently says `public documenation`; please correct this to `public documentation`.
```suggestion
documentation</a></p>
```
</issue_to_address>
### Comment 4
<location> `base_geoengine/tests/test_model.py:502` </location>
<code_context>
def test_search_within_for_retails_34(self):
retails = self.env["retail.machine"]
- zip_item = self.env["dummy.zip"].search([("city", "ilike", "Yens")])
+ zip_item = self.env["dummy.zip"].search([("city", "=", "Yens")], limit=1)
result = retails.search(
[("name", "ilike", "34"), ("the_point", "geo_within", zip_item.the_geom)]
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the test by asserting the setup data (zip_item) is found before using it
With the stricter domain, if `dummy.zip` ever stops containing a record with city `"Yens"`, `zip_item` will be empty and the test may give a misleading result. Add an explicit assertion like `self.assertTrue(zip_item, "Expected dummy.zip record for city 'Yens'")` before accessing `zip_item.the_geom` so the test fails early and clearly when the setup data is missing.
</issue_to_address>
### Comment 5
<location> `base_geoengine/tests/test_model.py:169` </location>
<code_context>
+
super().tearDownClass()
def test_create_multipolygon_wkt_format(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests covering invalid WKB/WKT inputs to reflect the new fallback behavior in value_to_shape
These changes add new behavior in `value_to_shape` (forcing `use_wkb=True`, catching `GEOSException`, logging failures, and falling back from WKB → WKT → `POINT(0 0)`). Please add tests here that cover: (1) a hex WKB string that raises `GEOSException` and still yields a valid geometry (ideally confirming the `POINT(0 0)` fallback when WKT is also invalid); (2) a non-geo string that fails both WKB and WKT and uses the fallback point; and (3) a valid WKT string to ensure forcing `use_wkb=True` doesn’t regress existing WKT-only inputs.
Suggested implementation:
```python
def test_create_multipolygon_wkt_format(self):
GeoModel = self.env["geo.model.test"]
wkt_multipolygon = "MULTIPOLYGON(((0 0,0 1,1 1,0 0)))"
rec = GeoModel.create(
{
"name": "wkt-multipolygon",
"the_geom": wkt_multipolygon,
}
)
# Ensure a valid geometry is created and it is a multipolygon
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "MultiPolygon")
def test_invalid_wkb_and_wkt_fallback_to_default_point(self):
GeoModel = self.env["geo.model.test"]
# This is an invalid hex WKB string and also not a valid WKT
invalid_wkb_and_wkt = "DEADBEEF"
rec = GeoModel.create(
{
"name": "invalid-wkb-wkt",
"the_geom": invalid_wkb_and_wkt,
}
)
# value_to_shape should catch GEOSException / parse errors and
# fall back to the default POINT(0 0) geometry
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "Point")
self.assertEqual(getattr(rec.the_geom, "x", None), 0)
self.assertEqual(getattr(rec.the_geom, "y", None), 0)
def test_non_geo_string_falls_back_to_default_point(self):
GeoModel = self.env["geo.model.test"]
non_geo_value = "this-is-not-a-geometry"
rec = GeoModel.create(
{
"name": "non-geo-string",
"the_geom": non_geo_value,
}
)
# Both WKB and WKT parsing should fail and we should still get
# the default POINT(0 0) geometry
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "Point")
self.assertEqual(getattr(rec.the_geom, "x", None), 0)
self.assertEqual(getattr(rec.the_geom, "y", None), 0)
def test_valid_wkt_with_wkb_first_parsing(self):
GeoModel = self.env["geo.model.test"]
# Valid WKT that should still be correctly parsed even though
# value_to_shape now tries WKB parsing first
wkt_point = "POINT(1 2)"
rec = GeoModel.create(
{
"name": "valid-wkt-point",
"the_geom": wkt_point,
}
)
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "Point")
self.assertEqual(getattr(rec.the_geom, "x", None), 1)
self.assertEqual(getattr(rec.the_geom, "y", None), 2)
def test_search_within_for_retails_34(self):
```
1. If `rec.the_geom` is not a GEOS geometry but a different wrapper, you may need to adapt the assertions to the existing test style in this file (for example, accessing coordinates via `rec.the_geom.coords[0]` or `rec.the_geom[0]` instead of `.x`/`.y` and possibly using another property instead of `.geom_type`).
2. Ensure that the `"the_geom"` field name and the `"name"` field exist on `geo.model.test`, or adjust them to match the model’s actual schema used elsewhere in `test_model.py`.
3. If your test suite uses a different fixture or factory to create `geo.model.test` records, replace direct `create` calls with that helper for consistency.
</issue_to_address>
### Comment 6
<location> `base_geoengine/README.rst:79-80` </location>
<code_context>
-installed.
-
-For a complete documentation please refer to the `public
+installed. For a complete documentation please refer to the `public
documenation <http://oca.github.io/geospatial/index.html>`__
Usage
</code_context>
<issue_to_address>
**issue (typo):** Fix typo "documenation" and consider improving the article before "complete documentation".
'documentation' is misspelled, and "the complete documentation" or just "complete documentation" would be more natural here.
```suggestion
installed. For complete documentation please refer to the `public
documentation <http://oca.github.io/geospatial/index.html>`__
```
</issue_to_address>
### Comment 7
<location> `base_geoengine/readme/INSTALL.md:18` </location>
<code_context>
installed.
-
For a complete documentation please refer to the [public
documenation](http://oca.github.io/geospatial/index.html)
</code_context>
<issue_to_address>
**issue (typo):** Correct the typo "documenation" and consider adjusting "a complete documentation".
This nearby line has the same typo: "documenation" → "documentation". Also consider changing "a complete documentation" to "the complete documentation" or simply "complete documentation" for better grammar.
</issue_to_address>
### Comment 8
<location> `base_geoengine/geo_convertion_helper.py:20` </location>
<code_context>
+ _logger.warning(_("Shapely or geojson are not available in the sys path")) # pylint: disable=prefer-env-translation
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object"""
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying `value_to_shape` by clarifying the `use_wkb` parameter semantics, extracting WKB/WKT parsing into helpers, and removing noisy inline comments to make the control flow easier to follow.
You can keep the new behavior while reducing complexity and making the flow easier to reason about.
### 1. Make `use_wkb` semantics explicit instead of overriding
Right now the parameter is misleading:
```python
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object"""
# always use_wkb for now
use_wkb = True
```
This makes the signature lie. To keep behavior but reduce cognitive load:
- Keep the parameter for API compatibility.
- Explicitly ignore it and document that it’s currently forced to WKB.
```python
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object.
NOTE: `use_wkb` is currently ignored and WKB is always tried first
for non-JSON strings. The parameter is kept for backward compatibility.
"""
# keep `use_wkb` in signature but avoid misleading overrides
_ = use_wkb # intentionally unused
if not value:
return wkt.loads("GEOMETRYCOLLECTION EMPTY")
...
```
This removes the hidden override while preserving the “always WKB first” behavior introduced by your change.
---
### 2. Extract WKB/WKT parsing into small helpers
You can linearize the control flow and unify error handling/logging by pushing the try/except into helpers:
```python
def _loads_wkb(value: str):
"""Try to load WKB; return geometry or None on failure."""
try:
return wkb.loads(value, hex=True)
except GEOSException:
_logger.warning("GEOSException during WKB load", exc_info=True)
except Exception:
_logger.warning("WKB conversion failed", exc_info=True)
return None
def _loads_wkt_with_fallback(value: str):
"""Load WKT, falling back to POINT(0 0) on failure."""
try:
return wkt.loads(value)
except Exception as e:
_logger.warning("Failed to parse WKT: %s", e)
return wkt.loads("POINT(0 0)")
```
Then `value_to_shape` becomes simpler and easier to scan:
```python
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object.
NOTE: `use_wkb` is currently ignored and WKB is always tried first
for non-JSON strings. The parameter is kept for backward compatibility.
"""
_ = use_wkb # intentionally unused
if not value:
return wkt.loads("GEOMETRYCOLLECTION EMPTY")
if isinstance(value, str):
if "{" in value:
geo_dict = geojson.loads(value)
return shape(geo_dict)
# always try WKB first
geom = _loads_wkb(value)
if geom is not None:
return geom
# WKB failed → WKT with POINT(0 0) fallback
return _loads_wkt_with_fallback(value)
if hasattr(value, "wkt"):
return value if isinstance(value, BaseGeometry) else wkt.loads(value.wkt)
raise TypeError(
_(
"Write/create/search geo type must be wkt/geojson "
"string or must respond to wkt"
)
)
```
This keeps all current semantics:
- WKB always attempted for non-JSON strings.
- On WKB failure, falls back to WKT.
- On WKT failure, returns `POINT(0 0)`.
---
### 3. Remove or relocate the large commented-out block
The long inline commented block about hash values and old behavior adds noise in the middle of the core path:
```python
# elif len(value) == 42 and "POINT" not in value and "(" not in value:
# not clear why use_wkb is not set correctly anymore.
# ...
# value = wkb.loads(value, hex=True)
# return value
```
If you want to preserve this rationale, move it out of the main function body, e.g.:
```python
# NOTE: Historical behavior:
# - There used to be a special case for 42-character strings without "POINT" or "("
# to handle hash-like WKB values such as
# "0101000000A4703D0AD7232C400AD7A3703D0A4840".
# - This is currently not supported; logic is kept here for reference:
# (see git history or design docs).
```
or into the module docstring/design notes. That keeps `value_to_shape` focused and easier to read.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+23
to
+24
| # always use_wkb for now | ||
| use_wkb = True |
There was a problem hiding this comment.
issue: Overriding use_wkb unconditionally makes the function parameter misleading and may mask configuration issues.
Forcing use_wkb = True here makes the argument meaningless and ignores caller intent. If this is intentional but temporary, either remove the parameter and update callers, or hide this behind a dedicated configuration flag. At minimum, add a clear TODO explaining why this override exists and under what conditions it should be reverted to avoid this silent behavior change becoming permanent.
Comment on lines
+38
to
+40
| except GEOSException as e: | ||
| _logger.warning("GEOSException:") | ||
| _logger.warning(e) |
There was a problem hiding this comment.
suggestion: Exception logging for GEOS/WKB failures could include more context to aid debugging.
Right now the GEOSException handler logs only a label and the exception. Please log some minimal context about the input (e.g. type and/or length) and pass exc_info=True so we get the traceback, for example:
_logger.warning(
"GEOSException while loading WKB (len=%s)",
len(value),
exc_info=True,
)The same improvement would help in the generic WKB conversion failure case as well.
| installed.</p> | ||
| <p>For a complete documentation please refer to the <a class="reference external" href="http://oca.github.io/geospatial/index.html">public | ||
| installed. For a complete documentation please refer to the <a class="reference external" href="http://oca.github.io/geospatial/index.html">public | ||
| documenation</a></p> |
There was a problem hiding this comment.
nitpick (typo): Minor spelling issue in the updated sentence.
The link text currently says public documenation; please correct this to public documentation.
Suggested change
| documenation</a></p> | |
| documentation</a></p> |
| def test_search_within_for_retails_34(self): | ||
| retails = self.env["retail.machine"] | ||
| zip_item = self.env["dummy.zip"].search([("city", "ilike", "Yens")]) | ||
| zip_item = self.env["dummy.zip"].search([("city", "=", "Yens")], limit=1) |
There was a problem hiding this comment.
suggestion (testing): Strengthen the test by asserting the setup data (zip_item) is found before using it
With the stricter domain, if dummy.zip ever stops containing a record with city "Yens", zip_item will be empty and the test may give a misleading result. Add an explicit assertion like self.assertTrue(zip_item, "Expected dummy.zip record for city 'Yens'") before accessing zip_item.the_geom so the test fails early and clearly when the setup data is missing.
|
|
||
| super().tearDownClass() | ||
|
|
||
| def test_create_multipolygon_wkt_format(self): |
There was a problem hiding this comment.
suggestion (testing): Add tests covering invalid WKB/WKT inputs to reflect the new fallback behavior in value_to_shape
These changes add new behavior in value_to_shape (forcing use_wkb=True, catching GEOSException, logging failures, and falling back from WKB → WKT → POINT(0 0)). Please add tests here that cover: (1) a hex WKB string that raises GEOSException and still yields a valid geometry (ideally confirming the POINT(0 0) fallback when WKT is also invalid); (2) a non-geo string that fails both WKB and WKT and uses the fallback point; and (3) a valid WKT string to ensure forcing use_wkb=True doesn’t regress existing WKT-only inputs.
Suggested implementation:
def test_create_multipolygon_wkt_format(self):
GeoModel = self.env["geo.model.test"]
wkt_multipolygon = "MULTIPOLYGON(((0 0,0 1,1 1,0 0)))"
rec = GeoModel.create(
{
"name": "wkt-multipolygon",
"the_geom": wkt_multipolygon,
}
)
# Ensure a valid geometry is created and it is a multipolygon
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "MultiPolygon")
def test_invalid_wkb_and_wkt_fallback_to_default_point(self):
GeoModel = self.env["geo.model.test"]
# This is an invalid hex WKB string and also not a valid WKT
invalid_wkb_and_wkt = "DEADBEEF"
rec = GeoModel.create(
{
"name": "invalid-wkb-wkt",
"the_geom": invalid_wkb_and_wkt,
}
)
# value_to_shape should catch GEOSException / parse errors and
# fall back to the default POINT(0 0) geometry
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "Point")
self.assertEqual(getattr(rec.the_geom, "x", None), 0)
self.assertEqual(getattr(rec.the_geom, "y", None), 0)
def test_non_geo_string_falls_back_to_default_point(self):
GeoModel = self.env["geo.model.test"]
non_geo_value = "this-is-not-a-geometry"
rec = GeoModel.create(
{
"name": "non-geo-string",
"the_geom": non_geo_value,
}
)
# Both WKB and WKT parsing should fail and we should still get
# the default POINT(0 0) geometry
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "Point")
self.assertEqual(getattr(rec.the_geom, "x", None), 0)
self.assertEqual(getattr(rec.the_geom, "y", None), 0)
def test_valid_wkt_with_wkb_first_parsing(self):
GeoModel = self.env["geo.model.test"]
# Valid WKT that should still be correctly parsed even though
# value_to_shape now tries WKB parsing first
wkt_point = "POINT(1 2)"
rec = GeoModel.create(
{
"name": "valid-wkt-point",
"the_geom": wkt_point,
}
)
self.assertTrue(rec.the_geom)
self.assertEqual(rec.the_geom.geom_type, "Point")
self.assertEqual(getattr(rec.the_geom, "x", None), 1)
self.assertEqual(getattr(rec.the_geom, "y", None), 2)
def test_search_within_for_retails_34(self):- If
rec.the_geomis not a GEOS geometry but a different wrapper, you may need to adapt the assertions to the existing test style in this file (for example, accessing coordinates viarec.the_geom.coords[0]orrec.the_geom[0]instead of.x/.yand possibly using another property instead of.geom_type). - Ensure that the
"the_geom"field name and the"name"field exist ongeo.model.test, or adjust them to match the model’s actual schema used elsewhere intest_model.py. - If your test suite uses a different fixture or factory to create
geo.model.testrecords, replace directcreatecalls with that helper for consistency.
Comment on lines
+79
to
80
| installed. For a complete documentation please refer to the `public | ||
| documenation <http://oca.github.io/geospatial/index.html>`__ |
There was a problem hiding this comment.
issue (typo): Fix typo "documenation" and consider improving the article before "complete documentation".
'documentation' is misspelled, and "the complete documentation" or just "complete documentation" would be more natural here.
Suggested change
| installed. For a complete documentation please refer to the `public | |
| documenation <http://oca.github.io/geospatial/index.html>`__ | |
| installed. For complete documentation please refer to the `public | |
| documentation <http://oca.github.io/geospatial/index.html>`__ |
| installed. | ||
|
|
||
| For a complete documentation please refer to the [public | ||
| documenation](http://oca.github.io/geospatial/index.html) |
There was a problem hiding this comment.
issue (typo): Correct the typo "documenation" and consider adjusting "a complete documentation".
This nearby line has the same typo: "documenation" → "documentation". Also consider changing "a complete documentation" to "the complete documentation" or simply "complete documentation" for better grammar.
| _logger.warning(_("Shapely or geojson are not available in the sys path")) # pylint: disable=prefer-env-translation | ||
|
|
||
|
|
||
| def value_to_shape(value, use_wkb=False): |
There was a problem hiding this comment.
issue (complexity): Consider simplifying value_to_shape by clarifying the use_wkb parameter semantics, extracting WKB/WKT parsing into helpers, and removing noisy inline comments to make the control flow easier to follow.
You can keep the new behavior while reducing complexity and making the flow easier to reason about.
1. Make use_wkb semantics explicit instead of overriding
Right now the parameter is misleading:
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object"""
# always use_wkb for now
use_wkb = TrueThis makes the signature lie. To keep behavior but reduce cognitive load:
- Keep the parameter for API compatibility.
- Explicitly ignore it and document that it’s currently forced to WKB.
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object.
NOTE: `use_wkb` is currently ignored and WKB is always tried first
for non-JSON strings. The parameter is kept for backward compatibility.
"""
# keep `use_wkb` in signature but avoid misleading overrides
_ = use_wkb # intentionally unused
if not value:
return wkt.loads("GEOMETRYCOLLECTION EMPTY")
...This removes the hidden override while preserving the “always WKB first” behavior introduced by your change.
2. Extract WKB/WKT parsing into small helpers
You can linearize the control flow and unify error handling/logging by pushing the try/except into helpers:
def _loads_wkb(value: str):
"""Try to load WKB; return geometry or None on failure."""
try:
return wkb.loads(value, hex=True)
except GEOSException:
_logger.warning("GEOSException during WKB load", exc_info=True)
except Exception:
_logger.warning("WKB conversion failed", exc_info=True)
return None
def _loads_wkt_with_fallback(value: str):
"""Load WKT, falling back to POINT(0 0) on failure."""
try:
return wkt.loads(value)
except Exception as e:
_logger.warning("Failed to parse WKT: %s", e)
return wkt.loads("POINT(0 0)")Then value_to_shape becomes simpler and easier to scan:
def value_to_shape(value, use_wkb=False):
"""Transforms input into a Shapely object.
NOTE: `use_wkb` is currently ignored and WKB is always tried first
for non-JSON strings. The parameter is kept for backward compatibility.
"""
_ = use_wkb # intentionally unused
if not value:
return wkt.loads("GEOMETRYCOLLECTION EMPTY")
if isinstance(value, str):
if "{" in value:
geo_dict = geojson.loads(value)
return shape(geo_dict)
# always try WKB first
geom = _loads_wkb(value)
if geom is not None:
return geom
# WKB failed → WKT with POINT(0 0) fallback
return _loads_wkt_with_fallback(value)
if hasattr(value, "wkt"):
return value if isinstance(value, BaseGeometry) else wkt.loads(value.wkt)
raise TypeError(
_(
"Write/create/search geo type must be wkt/geojson "
"string or must respond to wkt"
)
)This keeps all current semantics:
- WKB always attempted for non-JSON strings.
- On WKB failure, falls back to WKT.
- On WKT failure, returns
POINT(0 0).
3. Remove or relocate the large commented-out block
The long inline commented block about hash values and old behavior adds noise in the middle of the core path:
# elif len(value) == 42 and "POINT" not in value and "(" not in value:
# not clear why use_wkb is not set correctly anymore.
# ...
# value = wkb.loads(value, hex=True)
# return valueIf you want to preserve this rationale, move it out of the main function body, e.g.:
# NOTE: Historical behavior:
# - There used to be a special case for 42-character strings without "POINT" or "("
# to handle hash-like WKB values such as
# "0101000000A4703D0AD7232C400AD7A3703D0A4840".
# - This is currently not supported; logic is kept here for reference:
# (see git history or design docs).or into the module docstring/design notes. That keeps value_to_shape focused and easier to read.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixed first stress tests
Summary by Sourcery
Improve geo value conversion robustness and update test model registration for geoengine.
Bug Fixes:
Enhancements: