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.

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

More tests on the API #49

Merged
merged 6 commits into from
Sep 1, 2021
Merged

More tests on the API #49

merged 6 commits into from
Sep 1, 2021

Conversation

iskode
Copy link
Contributor

@iskode iskode commented Aug 17, 2021

Current tests focus on from_dataframe method. To have a better test coverage, I've written some test cases for __dataframe__ method. Note that this is a starting point. Added tests are not complete e.g Buffer tests are missing.

@iskode iskode changed the title More tests More tests on the API Aug 17, 2021
Copy link
Member

@kshitij12345 kshitij12345 left a comment

Choose a reason for hiding this comment

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

Made a couple of minor comments. Would love to know your thoughts.

Thanks! @iskode

protocol/pandas_implementation.py Outdated Show resolved Hide resolved
protocol/pandas_implementation.py Outdated Show resolved Hide resolved
@rgommers rgommers added the enhancement New feature or request label Aug 19, 2021
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.

Thanks @iskode. Modulo naming things, I think this test approach does make sense. The assert_frame_equal checks are more comprehensive, however a separate check that the structure of the object created by __dataframe__() is correct is useful.

protocol/pandas_implementation.py Outdated Show resolved Hide resolved
protocol/pandas_implementation.py Outdated Show resolved Hide resolved
@iskode
Copy link
Contributor Author

iskode commented Aug 24, 2021

I've found some issues with testing the buffer in case of categorical variables. Commented codes in the function assert_buffer_equal don't pass.

@iskode
Copy link
Contributor Author

iskode commented Aug 26, 2021

I've updated tests with recent merged PRs.
All tests passed and I left the categorical test commented out as suggested.

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 now, thanks @iskode

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

Successfully merging this pull request may close these issues.

3 participants