timsaucer commented on code in PR #1060:
URL:
https://github.com/apache/datafusion-python/pull/1060#discussion_r1991232153
##########
python/datafusion/context.py:
##########
@@ -468,6 +468,8 @@ class SessionContext:
See :ref:`user_guide_concepts` in the online documentation for more
information.
"""
+ _global_instance = None
+
Review Comment:
Why move this from the rust code to the python code?
##########
src/context.rs:
##########
@@ -308,7 +308,7 @@ impl PySessionContext {
#[classmethod]
#[pyo3(signature = ())]
- fn _global_ctx(_cls: &Bound<'_, PyType>) -> PyResult<Self> {
+ fn global_ctx(_cls: &Bound<'_, PyType>) -> PyResult<Self> {
Review Comment:
As you have it here where you moved the single entry over to the python
side, this method goes unused. I would recommend you leave this line as is, but
up in the python code you call this method instead of creating
`_global_instance`
##########
python/datafusion/context.py:
##########
@@ -498,6 +500,18 @@ def __init__(
self.ctx = SessionContextInternal(config, runtime)
+ @classmethod
+ def global_ctx(cls) -> "SessionContextInternal":
Review Comment:
I think we want to return the wrapper `SessionContext` since end users may
be getting this and would want to use the associated methods from the wrapper
classes. It would mean updating your methods in `read` accordingly.
--
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
