Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Exclude columns that are not one-dimensional #19

Merged
merged 4 commits into from
May 7, 2016
Merged

Exclude columns that are not one-dimensional #19

merged 4 commits into from
May 7, 2016

Conversation

KonstantinSchubert
Copy link
Contributor

I think I found a much better solution than the previous one.

It should not perform any unnecessary copies.

I am unsure if I should inline the get_nonscalar_columns() function or if I should even put it into module scope.

I will close the other pull request because I think this here is a much better solution.

@@ -169,14 +170,29 @@ def genchunks():
return convert_to_dataframe(arr)



Copy link
Collaborator

Choose a reason for hiding this comment

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

The whitespace here could be removed

@ibab
Copy link
Collaborator

ibab commented May 7, 2016

This is great!
Can you also add a few tests to https://github.com/ibab/root_pandas/blob/master/tests/test.py that make sure that nonscalar columns are dropped and that there are no errors?

@KonstantinSchubert
Copy link
Contributor Author

I hope this is fine?

@ibab ibab merged commit 28f2097 into scikit-hep:master May 7, 2016
@ibab
Copy link
Collaborator

ibab commented May 7, 2016

Yes, looks good!
We should think about how to make this work better with the flatten feature, but this is fine for now.

@KonstantinSchubert KonstantinSchubert deleted the alternative_approach2 branch May 7, 2016 19:45
@KonstantinSchubert
Copy link
Contributor Author

I agree regarding the flatten feature. I agree that requiring the user to specify the columns he wants to flatten would be a nice solution would be (also to avoid unexpected consequences of the flattening). If the user attempts to flatten an column with higher-dimensional arrays, we can simple throw an error.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants