Make item pickle smaller#1285
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1285 +/- ##
==========================================
+ Coverage 91.11% 91.12% +0.01%
==========================================
Files 51 51
Lines 6986 6994 +8
Branches 999 1001 +2
==========================================
+ Hits 6365 6373 +8
Misses 444 444
Partials 177 177 ☔ View full report in Codecov by Sentry. |
gadomski
left a comment
There was a problem hiding this comment.
The only limitation I can see is that if you have a resolved target object on your link, it will not be resolved with you roundtrip it to pickle and back.
This feels like the right behavior -- you (theoretically) can always re-resolve the object later. I'm ok with this change, but I'm not the target user here -- is there anyone that uses pystac in Dask-land that can verify that things are better w/ this fix. Maybe @TomAugspurger?
TomAugspurger
left a comment
There was a problem hiding this comment.
Won't have time to test this out for real, but in general I've found len(pickle.dumps(obj)) to be a good enough test for fixes like this.
Related Issue(s):
Description:
Not totally sure if this is a good idea, but I figured a PR would be a good place to hash that out. The only limitation I can see is that if you have a resolved target object on your link, it will not be resolved with you roundtrip it to pickle and back.
PR Checklist:
pre-commithooks pass locallyscripts/test)