Skip to content

Adjust parquet ArrowEngine to allow more easy subclass for writing#6505

Merged
jsignell merged 3 commits intodask:masterfrom
jorisvandenbossche:subclass-arrow-engine
Oct 19, 2020
Merged

Adjust parquet ArrowEngine to allow more easy subclass for writing#6505
jsignell merged 3 commits intodask:masterfrom
jorisvandenbossche:subclass-arrow-engine

Conversation

@jorisvandenbossche
Copy link
Member

Similar as @mariusvniekerk's PR #6211, but now also adding an overridable method for the "pandas->arrow" conversion in the write path.

cc @rjzamora I don't think this should have any impact on other subclasses?

Context: https://github.com/jsignell/dask-geopandas/pull/13

@jorisvandenbossche
Copy link
Member Author

cc @jsignell

Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor comments, otherwise this looks great!

index_cols = []
t = pa.Table.from_pandas(df, preserve_index=preserve_index, schema=schema)

t = cls._pandas_to_arrow_table(df, preserve_index, schema)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, but I prefer to include kwarg names.

@jsignell
Copy link
Member

Ok I am planning on merging this unless anyone has worries.

@jorisvandenbossche
Copy link
Member Author

This is ready to be merged, I think?

@jsignell jsignell merged commit 25a5db2 into dask:master Oct 19, 2020
@jsignell
Copy link
Member

Sorry I never hit the button @jorisvandenbossche!

@jorisvandenbossche jorisvandenbossche deleted the subclass-arrow-engine branch October 19, 2020 14:25
@jorisvandenbossche
Copy link
Member Author

No problem, thanks for merging!

kumarprabhu1988 pushed a commit to kumarprabhu1988/dask that referenced this pull request Oct 29, 2020
…ask#6505)

* Adjust parquet ArrowEngine to allow more easy subclass for the writing part

* add keyword names

* blacken
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants