You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "petern48 (via GitHub)" <gi...@apache.org> on 2025/08/14 22:24:57 UTC
[PR] [GH-2281] Geopandas: set ps option 'compute.ops_on_diff_frames' to True by default [sedona]
petern48 opened a new pull request, #2282:
URL: https://github.com/apache/sedona/pull/2282
## Did you read the Contributor Guide?
- Yes, I have read the [Contributor Rules](https://sedona.apache.org/latest/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/latest/community/develop/)
## Is this PR related to a ticket?
- Yes, and the PR name follows the format `[GH-XXX] my subject`. Closes #2281
## What changes were proposed in this PR?
- Set `ps.set_option("compute.ops_on_diff_frames", True)` in `__init__.py` by default. Enabled to True for testing. Spark 4.0.0 also changed the default to True, so this PR is primarily to change the behavior for older versions. Users shouldn't need to encounter errors for this being set to False by default. Setting it to False is primarily useful for sedona geopandas development to ensure we're not using unnecessary inefficiencies.
- Also, inside the tests, I replaced `with self.ps_allow_diff_frames()` with the built in `with ps.option_context("compute.ops_on_diff_frames", True)` from pyspark pandas
## How was this patch tested?
Updated and cleaned up tests
## Did this PR include necessary documentation updates?
- No, this PR does not affect any public API so no need to change the documentation.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2281] Geopandas: set ps option 'compute.ops_on_diff_frames' to True by default [sedona]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #2282:
URL: https://github.com/apache/sedona/pull/2282
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2281] Geopandas: set ps option 'compute.ops_on_diff_frames' to True by default [sedona]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2282:
URL: https://github.com/apache/sedona/pull/2282#discussion_r2284110791
##########
python/tests/geopandas/test_geopandas_base.py:
##########
@@ -33,6 +33,10 @@ class TestGeopandasBase(TestBase):
# -----------------------------------------------------------------------------
# # Utils
# -----------------------------------------------------------------------------
+ def setup_method(self):
+ # We enable this option by default for users, so we enable it by default for testing for developers.
+ # This option is useful in testing to catch inefficiencies in the code during development of geopandas.
Review Comment:
The comment on line 37 is confusing as it says 'we enable it by default for testing' but the code sets it to False. The comment should clarify that this is intentionally set to False for development testing to catch inefficiencies, contrary to the user default.
```suggestion
# Although this option is enabled by default for users, we intentionally set it to False during development testing
# to catch inefficiencies in the code when working with geopandas.
```
##########
python/tests/geopandas/test_geoseries.py:
##########
@@ -61,6 +61,7 @@ def setup_method(self):
),
]
)
+ super().setup_method()
Review Comment:
The `super().setup_method()` call should come before the test data setup, not after. Moving it to the beginning ensures proper initialization order.
```suggestion
```
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GH-2281] Geopandas: set ps option 'compute.ops_on_diff_frames' to True by default [sedona]
Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #2282:
URL: https://github.com/apache/sedona/pull/2282#discussion_r2284119880
##########
python/tests/geopandas/test_geopandas_base.py:
##########
@@ -33,6 +33,10 @@ class TestGeopandasBase(TestBase):
# -----------------------------------------------------------------------------
# # Utils
# -----------------------------------------------------------------------------
+ def setup_method(self):
+ # We enable this option by default for users, so we enable it by default for testing for developers.
+ # This option is useful in testing to catch inefficiencies in the code during development of geopandas.
Review Comment:
@petern48 please fix the comment
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org