chengchingwen / Pickle.jl

An experimental package for loading and saving object in Python Pickle format.
MIT License
50 stars 9 forks source link

Fix StridedView type piracy #28

Closed cossio closed 2 years ago

cossio commented 2 years ago

Closes #27.

codecov[bot] commented 2 years ago

Codecov Report

Merging #28 (e7f1409) into master (f70cd69) will decrease coverage by 1.17%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
- Coverage   80.70%   79.53%   -1.18%     
==========================================
  Files          14       13       -1     
  Lines        1016      997      -19     
==========================================
- Hits          820      793      -27     
- Misses        196      204       +8     
Impacted Files Coverage Δ
src/torch/torch_save.jl 78.94% <ø> (+2.28%) :arrow_up:
src/defaults.jl 0.00% <0.00%> (-91.67%) :arrow_down:
src/opcode/opcode.jl 15.00% <0.00%> (-3.19%) :arrow_down:
src/mt_table.jl 94.11% <0.00%> (-2.95%) :arrow_down:
src/deserializer.jl 55.46% <0.00%> (-0.35%) :arrow_down:
src/writearg.jl 84.44% <0.00%> (-0.34%) :arrow_down:
src/torch/torch_load.jl 92.94% <0.00%> (-0.17%) :arrow_down:
src/serializer.jl 85.83% <0.00%> (-0.12%) :arrow_down:
src/sparse.jl 100.00% <0.00%> (ø)
src/torch/torch.jl
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f70cd69...e7f1409. Read the comment docs.

cossio commented 2 years ago

The failure on Julia nightly looks unrelated to this PR.

CI is also failing on Julia 1.3 because it cannot satisfy all compats. Is there a reason to support v1.3 here, instead of the LTs 1.6?

cossio commented 2 years ago

Strided.jl now requires Julia >= 1.4, while we were attempting to do CI on Julia 1.3 here. So I bumped the minimum Julia version here to 1.4 instead of 1.3. With that CI should pass. @chengchingwen can you approve the CI run?

chengchingwen commented 2 years ago

Is there a reason to support v1.3 here, instead of the LTs 1.6?

Simply because I didn't remove that one, I think we could just drop the support below 1.6.

cossio commented 2 years ago

Okay, can you retrigger CI?

chengchingwen commented 2 years ago

Thanks!