Spaarsh commented on code in PR #1059:
URL:
https://github.com/apache/datafusion-python/pull/1059#discussion_r1991657860
##########
python/tests/test_wrapper_coverage.py:
##########
@@ -51,6 +58,8 @@ def missing_exports(internal_obj, wrapped_obj) -> None:
if isinstance(internal_attr, list):
assert isinstance(wrapped_attr, list)
for val in internal_attr:
+ if isinstance(val, str) and val.startswith("Raw"):
+ continue
Review Comment:
Actually, I did *not* fail when I removed `Expr` from the `__all__`. So
essentially, if some wrapper class is not included in `__all__` even then its
`Raw` class shall pass the test. To prevent this, I added the `assert
wrapper_class in dir(internal_obj)` before the `assert hasattr(wrapped_obj,
wrapper_class), f"Missing wrapper class: {wrapper_class} for Raw class: {val}"`
Now, if the `wrapper_class` isn't included in the `internal_obj`, then the
corresponding `Raw` shall fail:
```
FAILED
python/tests/test_wrapper_coverage.py::test_datafusion_missing_exports -
AssertionError: Missing wrapper class: Expr for Raw class: RawExpr
```
> I won't have time to review this again today, but I hope to get to it
tomorrow.
Sure. I'll see if I can find out any other gaps in my implementation till
then.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
