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

Support pandas StringArray and ArrowStringArray #441

Merged
merged 3 commits into from
Jul 5, 2024
Merged

Support pandas StringArray and ArrowStringArray #441

merged 3 commits into from
Jul 5, 2024

Conversation

Simon-Chenzw
Copy link
Contributor

@Simon-Chenzw Simon-Chenzw commented Jul 4, 2024

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Ensure PR doesn't contain untouched code reformatting: spaces, etc.
  • Run flake8 and fix issues.
  • Run pytest no tests failed. See https://clickhouse-driver.readthedocs.io/en/latest/development.html.

@coveralls
Copy link

Coverage Status

coverage: 96.252% (+0.001%) from 96.251%
when pulling f173c63 on Simon-Chenzw:master
into 59ea586 on mymarilyn:master.

@xzkostyan
Copy link
Member

It's that valgrind check is failed. I've tried to restart it twice with the same result.

total heap usage: 10,089,708 allocs, 9,941,721 frees, 3,025,851,569,279,572,588 bytes allocated

The same job in different PR is green.

total heap usage: 9,814,709 allocs, 9,374,178 frees, 3,025,851,569,241,805,533 bytes allocated

It seems that we have some leak, I've pushed valgrind with -s: https://github.com/mymarilyn/clickhouse-driver/actions/runs/9797329602

@Simon-Chenzw
Copy link
Contributor Author

The error occurred just because pyarrow was installed.
https://github.com/Simon-Chenzw/clickhouse-driver/actions/runs/9802697151
I'm going to delete it.

@Simon-Chenzw
Copy link
Contributor Author

Simon-Chenzw commented Jul 5, 2024

I tried debugging but got no results.
It seems beyond my ability.
Can we ignore the problem for now?

By the way, pandas has PDEP-10: PyArrow as a required dependency for default string inference implementation

PyArrow becomes a required runtime dependency starting with pandas 3.0

This might cause problems in the future.

@coveralls
Copy link

Coverage Status

coverage: 96.252% (+0.001%) from 96.251%
when pulling 40e9a6f on Simon-Chenzw:master
into 59ea586 on mymarilyn:master.

@xzkostyan xzkostyan merged commit 48403ed into mymarilyn:master Jul 5, 2024
246 of 444 checks passed
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.

Feature Request: Support pandas StringArray and ArrowStringArray
3 participants