paleolimbot commented on code in PR #1034:
URL: https://github.com/apache/sedona-db/pull/1034#discussion_r3556552195
##########
python/sedonadb/src/context.rs:
##########
@@ -165,6 +166,46 @@ impl InternalContext {
Ok(InternalDataFrame::new(df, self.runtime.clone()))
}
+ pub fn read_csv<'py>(
+ &self,
+ py: Python<'py>,
+ table_paths: Vec<String>,
+ has_header: bool,
+ delimiter: &str,
+ ) -> Result<InternalDataFrame, PySedonaError> {
+ let bytes = delimiter.as_bytes();
+ if bytes.len() != 1 {
+ return Err(PySedonaError::SedonaPython(format!(
+ "delimiter must be a single byte, got {delimiter:?}"
+ )));
+ }
+ let options = CsvReadOptions::new()
+ .has_header(has_header)
+ .delimiter(bytes[0]);
+ let df = wait_for_future(
+ py,
+ &self.runtime,
+ self.inner.ctx.read_csv(table_paths, options),
+ )??;
+ Ok(InternalDataFrame::new(df, self.runtime.clone()))
+ }
+
+ pub fn read_json<'py>(
+ &self,
+ py: Python<'py>,
+ table_paths: Vec<String>,
+ ) -> Result<InternalDataFrame, PySedonaError> {
+ // DataFusion reads newline-delimited JSON (one object per line).
Review Comment:
This has the same issues as read_csv()
##########
python/sedonadb/tests/io/test_read_csv_json.py:
##########
@@ -0,0 +1,105 @@
+# 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 pandas as pd
+import pandas.testing as pdt
+import pytest
+
+
+def _write(path, text):
+ path.write_text(text)
+ return str(path)
Review Comment:
All of our other tests use `with tempfile.TemporaryDirectory() as td` for
this, which cleans up the files (occasionally important on Windows)
##########
python/sedonadb/python/sedonadb/context.py:
##########
@@ -289,6 +289,71 @@ def read_parquet(
partitioning=partitioning,
)
+ def read_csv(
+ self,
+ table_paths: Union[str, Path, Iterable[str]],
+ *,
+ has_header: bool = True,
+ delimiter: str = ",",
+ ) -> DataFrame:
Review Comment:
Optional, but maybe we can avoid duplicating this from `.read.csv()` for
now? I left `read_parquet()` and `read_pyogrio()` for now but I'd like to move
docs and examples to use `sd.read(...)` since most of the time it can
effectively guess the format.
##########
python/sedonadb/src/context.rs:
##########
@@ -165,6 +166,46 @@ impl InternalContext {
Ok(InternalDataFrame::new(df, self.runtime.clone()))
}
+ pub fn read_csv<'py>(
+ &self,
+ py: Python<'py>,
+ table_paths: Vec<String>,
+ has_header: bool,
+ delimiter: &str,
+ ) -> Result<InternalDataFrame, PySedonaError> {
Review Comment:
I think we will want to expose this more similarly to `read_parquet()`,
which is implemented in rust/sedona and exposes its options as a
`HashMap<String, PyAny>`. There are a few things this does...one of them is
register any object stores that might be required, and allows cloud store
options to be set. The options as a hash map at this level makes it easier to
unify the Python and (future) R code paths...this might also automatically
expose them from `read()` more naturally without us having to remember to
update the options if new ones get added.
It would be nice to extract out any common read behaviour while we're here
(e.g., maybe using `session.state().file_format_factory()` to resolve a file
format) in rust/sedona so we can reuse it as needed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
