Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix empty dataframe not adding columns information #417

Merged
merged 2 commits into from
Mar 9, 2022
Merged

Fix empty dataframe not adding columns information #417

merged 2 commits into from
Mar 9, 2022

Conversation

igonro
Copy link
Contributor

@igonro igonro commented Mar 8, 2022

Closes #412

  • Write tests that would fail in src/danfojs-browser/...
  • Write tests that would fail in src/danfojs-node/...
  • Modify src/danfojs-base/... files to fix the issue
  • Run the tests again and check that the tests passed

Also I have tried to improve some tests in the "Empty NDFrame" group, because some of the existing tests were only testing DataFrame or Series creation instead of testing both.

Feel free to modify the code or add comments with the changes if you think something could be improved.

Thanks!

@igonro
Copy link
Contributor Author

igonro commented Mar 8, 2022

I suggest that both @risenW and @steveoni review this pull request, as it introduces some important changes and maybe I have missed something...

I did run the tests myself, but also I suggest that you run the tests again in order to be sure that these changes doesn't break anything.

@risenW risenW requested review from steveoni and risenW March 8, 2022 10:57
@risenW
Copy link
Member

risenW commented Mar 8, 2022

I suggest that both @risenW and @steveoni review this pull request, as it introduces some important changes and maybe I have missed something...

I did run the tests myself, but also I suggest that you run the tests again in order to be sure that these changes doesn't break anything.

Thanks @igonro We'll review ASAP

@igonro igonro mentioned this pull request Mar 8, 2022
Merged
2 tasks
risenW
risenW approved these changes Mar 9, 2022
View reviewed changes
Copy link
Member

@risenW risenW left a comment

Choose a reason for hiding this comment

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

LGTM @igonro Thanks for the awesome work!

@risenW risenW merged commit b712f70 into javascriptdata:dev Mar 9, 2022
@igonro igonro deleted the fix-empty-dataframe-not-adding-columns branch March 10, 2022 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@risenW risenW risenW approved these changes

@steveoni steveoni Awaiting requested review from steveoni

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Empty DataFrames are always created without columns information
2 participants
@igonro @risenW