Skip to content

feat: Replace cloudpickle with native Fory serialization #2417

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
Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: Replace cloudpickle with native Fory serialization #2417

wants to merge 2 commits into from

Conversation

esafak
Copy link
Contributor

@esafak esafak commented Jul 15, 2025

What does this PR do?

  • Removed cloudpickle dependency and related logic.
  • Replaced cloudpickle serialization with Fory's native serialization for unsupported types.
  • Updated serializers for Python arrays, NumPy arrays, and Objects to use native serialization.
  • Modified tests to reflect the removal of cloudpickle and use of native serialization.
  • Added global helper functions for testing to avoid issues with local function serialization.
  • Updated README.md with a note about bazel caching issues.

Related issues

Contributes to #2409

Notes

Next PickleSerializer will be removed bit by bit.

@esafak
feat: Replace cloudpickle with native Fory serialization
342cc4e 
* Removed cloudpickle dependency and related logic.
* Replaced cloudpickle serialization with Fory's native serialization for unsupported types.
* Updated serializers for Python arrays, NumPy arrays, and Objects to use native serialization.
* Modified tests to reflect the removal of cloudpickle and use of native serialization.
* Added global helper functions for testing to avoid issues with local function serialization.
* Updated README.md with a note about bazel caching issues.
@esafak esafak requested a review from chaokunyang as a code owner July 15, 2025 15:40
@esafak esafak marked this pull request as draft July 15, 2025 16:00
@esafak
Copy link
Contributor Author

esafak commented Jul 15, 2025

@chaokunyang I did not realize the xlang tests were being skipped in Python. I set ENABLE_CROSS_LANGUAGE_TESTS and now pytest complained that fixture 'data_file_path' not found is missing. I could not find a definition for it now or in the past. How is it supposed to work?

@chaokunyang
Copy link
Collaborator

chaokunyang commented Jul 15, 2025 •
edited
Loading

It's called from java CrossLanguageTest.java. Java serialize object into bytes and write to file, then execute python tests and pass that filepath as inputs

@esafak
Copy link
Contributor Author

esafak commented Jul 15, 2025

I think a comment the python xlang tests explain this would have helped, along with documentation of how to run them locally.

@chaokunyang
Copy link
Collaborator

chaokunyang commented Jul 16, 2025 •
edited
Loading

I think a comment the python xlang tests explain this would have helped, along with documentation of how to run them locally.

Yes, a comment would be helpful, thanks for this suggestion, I added a comment in #2420

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chaokunyang chaokunyang Awaiting requested review from chaokunyang chaokunyang is a code owner

At least 1 approving review is required to merge this pull request.

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

Successfully merging this pull request may close these issues.
2 participants
@esafak @chaokunyang