Skip to content

Add from_dict classmethod #112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 26, 2023
Merged

Conversation

MarcoGorelli
Copy link
Contributor

as discussed on the latest call

@MarcoGorelli
Copy link
Contributor Author

Thinking about this again, I'm not sure how this would work

In order for this to be possible, I think the following would probably be necessary:

  • constructors for Column
  • a way to generate a given DataFrame from a dict of Columns, which would probably require being able to get the underlying data from the columns

Let's talk about this more next time, but I'm not sure from_dict is the solution

@rgommers
Copy link
Member

constructors for Column

There should be a lot of functions that one can apply directly to columns, so you can get new columns from ops on existing ones. But agreed - probably also need to be able to construct columns from sequences and arrays for many use cases.

a way to generate a given DataFrame from a dict of Columns, which would probably require being able to get the underlying data from the columns

This one I don't understand - this should be a completely trivial operation, and no access of underlying data should be needed (a dataframe is simply an ordered collection of columns). Or am I missing your point?

@MarcoGorelli
Copy link
Contributor Author

Since we agreed in #118 that each library can have private methods to access the underlying data of its own columns, you're right that it is indeed trivial - e.g. in pandas we could do

    @classmethod
    def from_dict(cls, data: dict[str, PandasColumn]) -> PandasDataFrame:
        return cls(pd.DataFrame({label: column._series for label, column in data.items()}))

@MarcoGorelli
Copy link
Contributor Author

is this one OK, or does anything else need doing?

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

This LGTM indeed, in it goes. Thanks Marco.

@rgommers rgommers merged commit e0d387f into data-apis:main Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants