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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pickle compatibility is not given between pickle and cloudpickle => GitHub Action crash #778

Closed
Anselmoo opened this issue Apr 18, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@Anselmoo
Copy link
Contributor

馃悰馃悰 Bug Report

鈿楋笍 Current Behavior

By activating the pytest environment of GirHub Actions - package.yml, a compatibility problem shows up, as a result of incompatibility in the pickle protocols. The consequence is the CI test is crashed!

  1. https://github.com/Anselmoo/Hub/runs/2374960592?check_suite_focus=true

https://github.com/activeloopai/Hub/blob/f19d5663bbb79d8159121dbf4794d8ac6b4e3fcf/hub/api/tests/test_dataset.py#L179-L180

鈿欙笍 Environment

  • Python Version [3.6-3.8]
  • OS[UNIX, Linux, Windows]

馃敪 First Results

Every python version except the combination Windows + Python 3.8 needs Pickle5, vice versa, default pickle can be used

馃О Possible Solution (optional)

  1. Removed cloudpickle and only use pickle.dumb and pickle.loud
  2. Working case sensitive
@mynameisvinn
Copy link
Contributor

@Anselmoo this seems like an easy fix. Since a Dataset is easily serializable I am not sure why cloudpickle was used in the first place. Can you submit a PR and I will merge?

@Anselmoo
Copy link
Contributor Author

Anselmoo commented Apr 22, 2021

@mynameisvinn A fix of this issue is included in the PR #780 as part of the update of the GitHub Action pipeline. However, I can separate it.

@mynameisvinn
Copy link
Contributor

@Anselmoo mind separating the PRs? It will served as future documentation on the decision between pickle and cloudpickle :)

Submit PR and I'll merge!

This was referenced Apr 23, 2021
@Anselmoo
Copy link
Contributor Author

@mynameisvinn there is one more cloudpickle in the following test below

https://github.com/activeloopai/Hub/blob/27483e6c8dc2d45b4efcc478a6cdfbe6d2e0d532/hub/store/tests/test_s3_storage.py#L43-L47

If that is not necessary, we can completely remove it, even from the https://github.com/activeloopai/Hub/blob/master/requirements-dev.txt

AbhinavTuli pushed a commit that referenced this issue Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants