[SPARK-40809][SPARK-40780][FOLLOW-UP] Improve filter and alias testing coverage in python client #38278
Conversation
…g coverage for python client
| return DataFrame.withPlan(Read(table_name), cls.connect) # type: ignore | ||
|
|
||
| @classmethod | ||
| def _udf_mock(cls, *args, **kwargs): |
There was a problem hiding this comment.
nit:
| def _udf_mock(cls, *args, **kwargs): | |
| def _udf_mock(cls, *args, **kwargs) -> str: |
There was a problem hiding this comment.
done. I thought mypy checks such case but it seems not to be true on this line.
| @@ -47,6 +49,9 @@ def __init__(self) -> None: | |||
| def set_hook(self, name: str, hook: Any) -> None: | |||
| self.hooks[name] = hook | |||
|
|
|||
| def drop_hook(self, name: str) -> None: | |||
| self.hooks.pop(name) | |||
There was a problem hiding this comment.
should we catch the exceptions if self.hooks do not contains name?
There was a problem hiding this comment.
This is only for our testing and we have good control over it (e.g. make sure we only clean up things that registered for the testing) so I think it is ok.
| @classmethod | ||
| def _udf_mock(cls, *args, **kwargs): | ||
| return "internal_name" | ||
|
|
||
| @classmethod | ||
| def setUpClass(cls: Any) -> None: | ||
| cls.connect = MockRemoteSession() | ||
| cls.tbl_name = f"tbl{uuid.uuid4()}".replace("-", "") |
There was a problem hiding this comment.
We shouldn't actually use uuid here but call DROP IF EXISTS before and after running the tests. Since this PR doesn't target to address these, I am fine with doing separately too.
There was a problem hiding this comment.
ah yes.
I will follow up. Python side testing framework needs more refinement for sure.
|
Can one of the admins verify this patch? |
|
wired that seeing |
|
thinking my local mypy is different... |
amaliujia
force-pushed
the
test_where_as_in_python
branch
from
cdfa496
to
fd0867e
Compare
|
Merged to master. |
…g coverage in python client ### What changes were proposed in this pull request? 1. Refactor `test_connect_plan_only.py` and move common code to base class. 2. Test generated proto plan for `filter` and `alias`. ### Why are the changes needed? Improve python client testing coverage. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? UT Closes apache#38278 from amaliujia/test_where_as_in_python. Authored-by: Rui Wang <rui.wang@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
What changes were proposed in this pull request?
test_connect_plan_only.pyand move common code to base class.filterandalias.Why are the changes needed?
Improve python client testing coverage.
Does this PR introduce any user-facing change?
No
How was this patch tested?
UT