You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "itholic (via GitHub)" <gi...@apache.org> on 2024/09/12 09:15:12 UTC
[PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
itholic opened a new pull request, #48085:
URL: https://github.com/apache/spark/pull/48085
### What changes were proposed in this pull request?
This PR proposes to add API compatibility check between Classic and Connect.
This PR also includes updating both APIs to the same signature.
### Why are the changes needed?
APIs supported on both Spark Connect and Spark Classic should guarantee the same signature, such as argument and return types.
### Does this PR introduce _any_ user-facing change?
No, it is a test to prevent future API behavior inconsistencies between Classic and Connect.
### How was this patch tested?
Added UTs.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #48085:
URL: https://github.com/apache/spark/pull/48085#issuecomment-2370599343
Merged to master. Let me create an umbrella JIRA to address the rest of tasks.
Thanks all for the review!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1770735191
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
+ from pyspark.sql.connect.dataframe import DataFrame as ConnectDataFrame
+ from pyspark.sql.classic.column import Column as ClassicColumn
+ from pyspark.sql.connect.column import Column as ConnectColumn
+
+ def get_public_methods(cls):
+ """Get public methods of a class."""
+ return {
+ name: method
+ for name, method in inspect.getmembers(cls, predicate=inspect.isfunction)
+ if not name.startswith("_")
+ }
+
+ def compare_method_signatures(classic_cls, connect_cls, cls_name):
+ """Compare method signatures between classic and connect classes."""
+ classic_methods = get_public_methods(classic_cls)
+ connect_methods = get_public_methods(connect_cls)
+
+ common_methods = set(classic_methods.keys()) & set(connect_methods.keys())
+
+ for method in common_methods:
+ classic_signature = inspect.signature(classic_methods[method])
+ connect_signature = inspect.signature(connect_methods[method])
+
+ self.assertEqual(
+ classic_signature,
+ connect_signature,
+ f"Signature mismatch in {cls_name} method '{method}'\n"
+ f"Classic: {classic_signature}\n"
+ f"Connect: {connect_signature}",
+ )
+
+ # DataFrame API signature comparison
+ compare_method_signatures(ClassicDataFrame, ConnectDataFrame, "DataFrame")
+
+ # Column API signature comparison
+ compare_method_signatures(ClassicColumn, ConnectColumn, "Column")
Review Comment:
Just added!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on PR #48085:
URL: https://github.com/apache/spark/pull/48085#issuecomment-2347896416
I'm wondering if we should mention the parameter renaming (fix) in the user-facing change section of the PR description
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "allisonwang-db (via GitHub)" <gi...@apache.org>.
allisonwang-db commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1769325573
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
+ from pyspark.sql.connect.dataframe import DataFrame as ConnectDataFrame
+ from pyspark.sql.classic.column import Column as ClassicColumn
+ from pyspark.sql.connect.column import Column as ConnectColumn
+
+ def get_public_methods(cls):
+ """Get public methods of a class."""
+ return {
+ name: method
+ for name, method in inspect.getmembers(cls, predicate=inspect.isfunction)
+ if not name.startswith("_")
+ }
+
+ def compare_method_signatures(classic_cls, connect_cls, cls_name):
+ """Compare method signatures between classic and connect classes."""
+ classic_methods = get_public_methods(classic_cls)
+ connect_methods = get_public_methods(connect_cls)
+
+ common_methods = set(classic_methods.keys()) & set(connect_methods.keys())
+
+ for method in common_methods:
+ classic_signature = inspect.signature(classic_methods[method])
+ connect_signature = inspect.signature(connect_methods[method])
+
+ self.assertEqual(
+ classic_signature,
+ connect_signature,
+ f"Signature mismatch in {cls_name} method '{method}'\n"
+ f"Classic: {classic_signature}\n"
+ f"Connect: {connect_signature}",
+ )
+
+ # DataFrame API signature comparison
+ compare_method_signatures(ClassicDataFrame, ConnectDataFrame, "DataFrame")
+
+ # Column API signature comparison
+ compare_method_signatures(ClassicColumn, ConnectColumn, "Column")
Review Comment:
How about methods in SparkSession?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1770731167
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
+ from pyspark.sql.connect.dataframe import DataFrame as ConnectDataFrame
+ from pyspark.sql.classic.column import Column as ClassicColumn
+ from pyspark.sql.connect.column import Column as ConnectColumn
+
+ def get_public_methods(cls):
+ """Get public methods of a class."""
+ return {
+ name: method
+ for name, method in inspect.getmembers(cls, predicate=inspect.isfunction)
+ if not name.startswith("_")
+ }
+
+ def compare_method_signatures(classic_cls, connect_cls, cls_name):
+ """Compare method signatures between classic and connect classes."""
+ classic_methods = get_public_methods(classic_cls)
+ connect_methods = get_public_methods(connect_cls)
+
+ common_methods = set(classic_methods.keys()) & set(connect_methods.keys())
+
+ for method in common_methods:
+ classic_signature = inspect.signature(classic_methods[method])
+ connect_signature = inspect.signature(connect_methods[method])
+
+ self.assertEqual(
+ classic_signature,
+ connect_signature,
+ f"Signature mismatch in {cls_name} method '{method}'\n"
+ f"Classic: {classic_signature}\n"
+ f"Connect: {connect_signature}",
+ )
+
+ # DataFrame API signature comparison
+ compare_method_signatures(ClassicDataFrame, ConnectDataFrame, "DataFrame")
+
+ # Column API signature comparison
+ compare_method_signatures(ClassicColumn, ConnectColumn, "Column")
Review Comment:
Yeah, we should add more public APIs including SparkSession for sure
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1761439052
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
Review Comment:
@itholic can we add some test to verify that this keeps functioning? Basically we need some a couple of broken signatures.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1770730445
##########
python/pyspark/sql/connect/dataframe.py:
##########
@@ -511,7 +511,7 @@ def drop(self, *cols: "ColumnOrName") -> ParentDataFrame: # type: ignore[misc]
session=self._session,
)
- def filter(self, condition: Union[Column, str]) -> ParentDataFrame:
Review Comment:
I think he meant to keep the type hints because you're changing it to `ColumnOrName` which means a column name. However, the actual argument it takes is a SQL expression string.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1770730767
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
+ from pyspark.sql.connect.dataframe import DataFrame as ConnectDataFrame
+ from pyspark.sql.classic.column import Column as ClassicColumn
+ from pyspark.sql.connect.column import Column as ConnectColumn
+
+ def get_public_methods(cls):
+ """Get public methods of a class."""
+ return {
+ name: method
+ for name, method in inspect.getmembers(cls, predicate=inspect.isfunction)
+ if not name.startswith("_")
+ }
+
+ def compare_method_signatures(classic_cls, connect_cls, cls_name):
+ """Compare method signatures between classic and connect classes."""
+ classic_methods = get_public_methods(classic_cls)
+ connect_methods = get_public_methods(connect_cls)
+
+ common_methods = set(classic_methods.keys()) & set(connect_methods.keys())
Review Comment:
Got it! Added `check_missing_methods` for missing methods testing
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1762408169
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
Review Comment:
can you add it into `dev/sparktestsupport/modules.py` so the tests actually run
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1770730284
##########
python/pyspark/sql/connect/dataframe.py:
##########
@@ -511,7 +511,7 @@ def drop(self, *cols: "ColumnOrName") -> ParentDataFrame: # type: ignore[misc]
session=self._session,
)
- def filter(self, condition: Union[Column, str]) -> ParentDataFrame:
Review Comment:
no, I think we should change it in all 3 `dataframe.py` to `def filter(self, condition: Union[Column, str])`.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1770731030
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
Review Comment:
Yeah I think we should add. Let me update
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1764749285
##########
python/pyspark/sql/connect/dataframe.py:
##########
@@ -511,7 +511,7 @@ def drop(self, *cols: "ColumnOrName") -> ParentDataFrame: # type: ignore[misc]
session=self._session,
)
- def filter(self, condition: Union[Column, str]) -> ParentDataFrame:
Review Comment:
`str` type here means a SQL expression, not a column name
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1761439747
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
Review Comment:
What else should we include here? SparkSession? ... Basically anything we consider public 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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1761440853
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
+ from pyspark.sql.connect.dataframe import DataFrame as ConnectDataFrame
+ from pyspark.sql.classic.column import Column as ClassicColumn
+ from pyspark.sql.connect.column import Column as ConnectColumn
+
+ def get_public_methods(cls):
+ """Get public methods of a class."""
+ return {
+ name: method
+ for name, method in inspect.getmembers(cls, predicate=inspect.isfunction)
+ if not name.startswith("_")
+ }
+
+ def compare_method_signatures(classic_cls, connect_cls, cls_name):
+ """Compare method signatures between classic and connect classes."""
+ classic_methods = get_public_methods(classic_cls)
+ connect_methods = get_public_methods(connect_cls)
+
+ common_methods = set(classic_methods.keys()) & set(connect_methods.keys())
Review Comment:
You should also check for missing methods.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #48085:
URL: https://github.com/apache/spark/pull/48085#issuecomment-2369929967
let's create a umbrella JIRA, and cover other API like functions.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic closed pull request #48085: [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect
URL: https://github.com/apache/spark/pull/48085
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1770733748
##########
python/pyspark/sql/connect/dataframe.py:
##########
@@ -511,7 +511,7 @@ def drop(self, *cols: "ColumnOrName") -> ParentDataFrame: # type: ignore[misc]
session=self._session,
)
- def filter(self, condition: Union[Column, str]) -> ParentDataFrame:
Review Comment:
Got it. Thanks for the clarification!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #48085:
URL: https://github.com/apache/spark/pull/48085#issuecomment-2345717907
cc @HyukjinKwon @hvanhovell
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1758064565
##########
python/pyspark/sql/connect/dataframe.py:
##########
@@ -727,8 +727,8 @@ def _convert_col(df: ParentDataFrame, col: "ColumnOrName") -> Column:
session=self._session,
)
- def limit(self, n: int) -> ParentDataFrame:
- res = DataFrame(plan.Limit(child=self._plan, limit=n), session=self._session)
+ def limit(self, num: int) -> ParentDataFrame:
Review Comment:
Good catch!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1761439747
##########
python/pyspark/sql/tests/test_connect_compatibility.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import unittest
+from pyspark.testing.sqlutils import ReusedSQLTestCase
+
+
+class ConnectCompatibilityTestsMixin:
+ def test_signature_comparison_between_classic_and_connect(self):
+ """Compare method signatures between ClassicDataFrame and ConnectDataFrame."""
+ import inspect
+ from pyspark.sql.classic.dataframe import DataFrame as ClassicDataFrame
Review Comment:
What else should we include here? SparkSession? ...
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1764755034
##########
python/pyspark/sql/connect/dataframe.py:
##########
@@ -1252,7 +1252,7 @@ def intersectAll(self, other: ParentDataFrame) -> ParentDataFrame:
res._cached_schema = self._merge_cached_schema(other)
return res
- def where(self, condition: Union[Column, str]) -> ParentDataFrame:
Review Comment:
ditto
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-49609][PYTHON][CONNECT] Add API compatibility check between Classic and Connect [spark]
Posted by "xinrong-meng (via GitHub)" <gi...@apache.org>.
xinrong-meng commented on code in PR #48085:
URL: https://github.com/apache/spark/pull/48085#discussion_r1758064051
##########
python/pyspark/sql/connect/dataframe.py:
##########
@@ -2189,18 +2189,18 @@ def cb(ei: "ExecutionInfo") -> None:
return DataFrameWriterV2(self._plan, self._session, table, cb)
- def mergeInto(self, table: str, condition: Column) -> MergeIntoWriter:
+ def mergeInto(self, table: str, condition: Column) -> "MergeIntoWriter":
def cb(ei: "ExecutionInfo") -> None:
self._execution_info = ei
return MergeIntoWriter(
self._plan, self._session, table, condition, cb # type: ignore[arg-type]
)
- def offset(self, n: int) -> ParentDataFrame:
- return DataFrame(plan.Offset(child=self._plan, offset=n), session=self._session)
+ def offset(self, num: int) -> ParentDataFrame:
Review Comment:
Good catch!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org