Skip to content

ARROW-2209: [Python] Partition columns are not correctly loaded in schema of ParquetDataset#1656

Closed
xhochy wants to merge 3 commits intoapache:masterfrom
xhochy:ARROW-2209
Closed

ARROW-2209: [Python] Partition columns are not correctly loaded in schema of ParquetDataset#1656
xhochy wants to merge 3 commits intoapache:masterfrom
xhochy:ARROW-2209

Conversation

@xhochy
Copy link
Member

@xhochy xhochy commented Feb 25, 2018

No description provided.

@xhochy
Copy link
Member Author

xhochy commented Feb 25, 2018

Necessary fixes for the pyarrow side of dask/dask#3171

def get_field_index(self, name):
return self.schema.GetFieldIndex(tobytes(name))

def add_field(self, int i, Field field):
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like just a wrapping of the C++ API. Is there an API that we can provide that'll be more familiar to Python users? How about insert/append/remove similar to lists?

Copy link
Member

Choose a reason for hiding this comment

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

Either way I think these methods need to return new objects, so Schema is not mutable like list, though

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. I don't think the semantics need to be identical, but I think the spelling can be improved.

Copy link
Member Author

Choose a reason for hiding this comment

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

As this is most similar to a list, I would use the function names as defined there but would do some adjustments, e.g.:

  • Always return a new object
  • Use indices instead of actual values, i.e. "remove(x) would be remove(i)".

@xhochy
Copy link
Member Author

xhochy commented Feb 27, 2018

Adjusted the API to be more like a list().

Copy link
Member Author

@xhochy xhochy left a comment

Choose a reason for hiding this comment

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

+1

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