Changes between Initial Version and Version 1 of Ticket #28444, comment 50


Ignore:
Timestamp:
Sep 4, 2019, 11:51:54 PM (3 years ago)
Author:
Nils Bruin
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #28444, comment 50

    initial v1  
    99I'm rather strongly against misusing `str` for binary data. That's not what it's for! And what you can see in this ticket already, is that signalling intent properly in pickle files is of utmost importance for forward/backward compatibility option. The observation that it ends up shorter as a `str` could be something stupid as the sequence having a UTF-8 representation that happens to be surprisingly short. If you think the pickle format for binary can be improved, then patch pickle. I think `Matrix_gfpn_dense` should just have the interface that makes most sense for it. It can also have a custom `__reduce__` that encodes its defining data in a sensible way (it probably already has); possible zipping or otherwise compressing the data if that is appropriate (I don't think it is -- compression can always be done in a separate round)
    1010
    11 `Matrix_gfpn_dense` is undoubtedly a cython class, so it must already use a `__reduce__`. That should give us a hook for `copyreg` to insert an "unpickled arguments preprocessing step", so the matrix format doesn't have to be burdened with compatibility features in its API.
    12 
    13 > I would like to solve one case in a new ticket (namely `Matrix_gfpn_dense`) without copyreg. If further cases pop up (perhaps in the pickle jar?), we may tackle them here with copyreg. I don't know how difficult it is.
    14 
    15 We'll see. I think having the shim in is worth considering too.
     11`Matrix_gfpn_dense` is very easy to fix: they are ALREADY unpickled using a helper function `mtx_unpickle`. If that would accept as Data both `bytes` and `str` (which it can then transform in `bytes` using the `latin1` encoder), you'd be done. That doesn't affect the normal API of the class at all.
    1612
    1713> If meataxe was a standard package (there are reasons to make it standard, btw.), `Matrix_gfpn_dense` should certainly become part of the pickle jar.