Add csv & parquet write functions and toPandas to experimental PySpark API

[TomBurdge]

As discussed with @Tishj here I have added a few features to the experimental DuckDB PySpark API:

• Add csv method to Spark DataFrameWriter
• Add parquet method to Spark DataFrameWriter
• toPandas method for the Spark DataFrame

I have added accompanying unit tests.

• CSV tests are the existing DuckDBPyRelation unit tests for writing a csv modified for the PySpark syntax.
• Parquet unit tests are more basic, as there are less implemented arguments to test are working.
• toPandas test has been added to the test_spark_pandas_dataframe.py tests.

First contribution so I have tried to follow the contribution guidelines.
I have run the unit tests and make reformatted. The main tests passed but there was a failure for the sqlite wrapper; I haven't changed anything that has anything to do with this so it might be my local setup.
The bits in pythonpckg readme about stubs were a bit over my head; happy to fix if I haven't done anything right, but might need a point in the right direction😊.

[Tishj]

I think we don't want to have a dependency on pandas, what we do in other places is set "PandasDataFrame" as the return type annotation and then inside the method do the actual import - so it only fails to import when toPandas is used if it's not installed.

[TomBurdge]

Good point. Fixed here: b849a19fc299b6e87a0069e1aacb82630a6671a3

[Tishj]

Thanks, though I think the type annotation needs to be in quotes still?
Otherwise this is referencing a type that doesn't exist, can't imagine the type checker to be happy with that?

[TomBurdge]

Not fixed properly yet^

[TomBurdge]

Ok, I have tried to fix it in 2c5e7b690e61c22c4eec18db15706111fce858f7.
I have tried to make it mirror how it works in the session.py with the type checking.

[Tishj]

Can we please use the exact prototype as PySpark?
https://spark.apache.org/docs/latest/api/python/reference/pyspark.sql/api/pyspark.sql.DataFrameWriter.csv.html

This is missing type annotation for the parameters

[TomBurdge]

eea76cf5c174b09d795a4e22e9e791698b8eaaba

[Tishj]

pytest has a tmp_path fixture, which should be used instead of manually using tempfile
(example from another test)

    def test_read_csv_glob(self, tmp_path, create_temp_csv):
        file1_path, file2_path = create_temp_csv

        # Use the temporary file paths to read CSV files
        con = duckdb.connect()
        rel = con.read_csv(f'{tmp_path}/file*.csv')
        res = con.sql("select * from rel order by all").fetchall()
        assert res == [(1,), (2,), (3,), (4,), (5,), (6,)]

[TomBurdge]

Makes sense, fixed in ff1e65087754405fc15f632c20d2c5bb696e4fcb and 0f6cc01f0b398f236eaf50bb6be2912efde70f89

[Tishj]

This is a bit of a nitpick but there's a good bit of repetition in the dataframes used to test, with a fixture we can avoid duplicating these and making it more explicit where these are repeated.
3+ repetitions (and the pieces of code are standalone) are usually a good sign it should be a fixture 👍

test_spark_join.py has a good example of this

@pytest.fixture
def dataframe_a(spark):
    emp = [
        (1, "Smith", -1, "2018", "10", "M", 3000),
        (2, "Rose", 1, "2010", "20", "M", 4000),
        (3, "Williams", 1, "2010", "10", "M", 1000),
        (4, "Jones", 2, "2005", "10", "F", 2000),
        (5, "Brown", 2, "2010", "40", "", -1),
        (6, "Brown", 2, "2010", "50", "", -1),
    ]
    empColumns = ["emp_id", "name", "superior_emp_id", "year_joined", "emp_dept_id", "gender", "salary"]
    dataframe = spark.createDataFrame(data=emp, schema=empColumns)
    yield dataframe


@pytest.fixture
def dataframe_b(spark):
    dept = [("Finance", 10), ("Marketing", 20), ("Sales", 30), ("IT", 40)]
    deptColumns = ["dept_name", "dept_id"]
    dataframe = spark.createDataFrame(data=dept, schema=deptColumns)
    yield dataframe


class TestDataFrameJoin(object):
    def test_inner_join(self, dataframe_a, dataframe_b):
        df = dataframe_a.join(dataframe_b, dataframe_a.emp_dept_id == dataframe_b.dept_id, "inner")
        ...

[TomBurdge]

Thanks for the suggestion. Wasn't sure how to parametrize fixtures, but figured it out: c394de3de3ed82470f75bd7dc98a392024223995

[Tishj]

This is unused, no?
Same goes for connect

[TomBurdge]

Ah yes, there must be something overriding my normal VSCode settings as normally that would point this out: fcea55801ce6e13e7962be7f424236f9e7fe1154

[Tishj]

Thanks for the work you've done here!
Looks great, just a few changes I'd like to see 👍

[TomBurdge]

I'm very confused by the git hashes you leave in your comments, I do not see any changes reflected in this PR, and thus I can't review your changes.
Please apply the changes to this branch 👍

Ah sorry, I have applied the changes and pushed to this branch but I can see now that the hashes are a bit unhelpful there. The changes are now applied, I think you should be able to see the updates.

(And happy to do anything to make the review easier, if it's not showing or whatever else just lmk)

[TomBurdge]

Hi @Tishj let me know if there's anything further I can do to help with this pull request and I am happy to! All changes requested should be pushed to this branch now.

[Tishj]

As mentioned in earlier comments:

    def csv(
        self,
        path: str,
        mode: Optional[str] = None,
        compression: Optional[str] = None,
        sep: Optional[str] = None,
        quote: Optional[str] = None,
        escape: Optional[str] = None,
        header: Optional[Union[bool, str]] = None,
        nullValue: Optional[str] = None,
        escapeQuotes: Optional[Union[bool, str]] = None,
        quoteAll: Optional[Union[bool, str]] = None,
        dateFormat: Optional[str] = None,
        timestampFormat: Optional[str] = None,
        ignoreLeadingWhiteSpace: Optional[Union[bool, str]] = None,
        ignoreTrailingWhiteSpace: Optional[Union[bool, str]] = None,
        charToEscapeQuoteEscaping: Optional[str] = None,
        encoding: Optional[str] = None,
        emptyValue: Optional[str] = None,
        lineSep: Optional[str] = None,

https://spark.apache.org/docs/latest/api/python/_modules/pyspark/sql/readwriter.html#DataFrameWriter.csv

[TomBurdge]

Ah thank you, I have fixed this now - type hints are identical in the latest version of this branch.

[Tishj]

Seems you left a TODO in there?

[TomBurdge]

yes - already checked it and was happy. Thanks for noticing, I have removed this comment in the latest of this branch.

[Tishj]

Thanks, I left a few comments

[Tishj]

Thanks, looks good to me 👍