add support for object values

[paroga]

Pickeld values are stored with a "|O" TypeStr.

Todo: (added by owner)

• If it is simple enough, add an example that shows an array being serialized/deserialized through pickle.
• NpyHeader::item_size should become an Option. This is an internal change and doesn't impact users.
  • (we could also keep it what it is and then add a has_known_size bool, but that's exactly the same in terms of storage, and would only have a performance benefit on NpyData::get_unchecked...)
• NpyReader has seek_to and read_at methods. NpyData has get, get_unchecked. These cannot really work for |O and we need to think of what they should do (Err or panic?). NpyData::from_bytes has an assertion that would need adjustment. Luckily these all use NpyHeader::item_size.
  • Option 1: everybody panics (except from_bytes)
  • Option 2: get_unchecked is UB, the rest return Err (except from_bytes)
  • Or something else....
• (exphp): type_matchup_docs should mention |O.
• (exphp): TypeStr::size_field should mention |O just like how it mentions U.
• (exphp): general adjustments
• (exphp): pickle array example and not just a scalar.
• (exphp): ensure that Display handles O correctly
• (exphp): DType::has_variable_size
• (exphp) Test case: Header containing an Object shouldn't error
• (exphp) Remove NpyData::from_bytes's panic. It is unfair even for fixed size types.
• (exphp) "a4" without the | endianness marker should be supported #81 . It affects O too.

[ExpHP]

Dang it, I don't get how I keep missing my GitHub notifications! 😅
I'll take a look

[ExpHP]

Should we be erroring on |O4?

[paroga]

the code will bail with a SyntaxError now

[ExpHP]

I'm not sure I understand. The PR says it adds support for "Empty" types, but then talks about pickled objects. It would seem to me that |O likely means "object" rather than empty..?

Part of me is tempted to say that if |O has a dynamic length, then perhaps the "size" of a format string should be Option... though this would be a breaking change.

I'm interested in seeing an example of how this could be used. In particular I am wondering if the downstream code would have to be using TypeStr, or if this addition allows other parts npyz to work with arrays that they currently cannot.

[paroga]

I'm not sure I understand. The PR says it adds support for "Empty" types, but then talks about pickled objects. It would seem to me that |O likely means "object" rather than empty..?

Good point. I renamed it to Object.

Part of me is tempted to say that if |O has a dynamic length, then perhaps the "size" of a format string should be Option... though this would be a breaking change.

I personally don't see the need to make it an option. If you want to support Object you'll need explicit code for it anyway. For everybody else if would just introduce a bunch of (ugly) .unwrap() with no real benefit.

I'm interested in seeing an example of how this could be used. In particular I am wondering if the downstream code would have to be using TypeStr, or if this addition allows other parts npyz to work with arrays that they currently cannot.

My downstream code uses npyz to parse the "header" and then passes the rest to serde-pickle. npyz won't be able to parse more data with this change, but it allows downstream to reuse the parsing capabilities in a clean way. Let me know if you want/need to see an actual working example.

[ExpHP]

I see! If I'm picturing correctly how you're using it, it sounds like it might actually be pretty simple to write an example that can Deserialize/Serialize an array of pickled objects. This could be a great example to have.

[paroga]

i've added an example with serde_pickle and made NpyHeader::item_size return an Option

[ExpHP]

Changing the public DType::num_bytes to return Result<Option< is a curious choice. Looks like something an AI would do lol. I do not expect it should be harmful to overload the meaning of None here to mean either "above max size" or "unknown".

I think I've got all the pieces I need now, I'll take it from here.

[ExpHP]

Oh, dang, I wish you had kept those as separate commits. c0f214a would be a better starting point for me but IDK if it will correctly recognize this PR as merged (or you as contributor) if I start from there now.

I'll figure something out.

[ExpHP]

serde_pickle is 1.0 and looks safe to have as a public dependency, so I'm actually going to add a feature for this and provide adaptors anyone can use.

[ExpHP]

...........................................

Uhhhh.................

So.... presumably in order to save space on declarations of types and attribute names, whenever serializing an array involves any pickling at all, it pickles the entire ndarray as a single object. It still has a header giving a shape and everything, but after the header there is only a single pickle, not one pickle for each item. This is also true for structured dtypes with an Object hiding somewhere inside them.

Which, like....... all of this completely breaks the entire API of this crate, and so just about nearly everything I worked on today to add to this PR has been a colossal waste of time. 😅 TypeRead/TypeWrite adapters for serde_pickle, npyz trait impls for serde_pickle Value, and plenty of documentation is all garbo.

It's looking like the feature really can only be the following:

• Parsing/rendering of "|O"
• {TypeStr,DType}::uses_pickle methods. When true, the entire array is pickled.
• Errors on pretty much all functions that read or write items in an array (not just those involving random access), since these all rely on the assumption that "# of things we can read == product of array shape". DType and Header are the only things safe to use.

Uggg

[paroga]

Changing the public DType::num_bytes to return Result<Option< is a curious choice.

Since the field is an Option< it was the natural way for me to add a clear way between error and not existing. I tried it another way first, but it was even worse IMO.

serde_pickle is 1.0

yes, but seams not maintained - i didn't get any response for my PR so far

nearly everything I worked on today to add to this PR has been a colossal waste of time

sorry to hear that and not being more clear about how it works upfront.

i'm not sure how you see the scope for this crate, but adding full decoding support for pickled values seams like a completely different API to me. i'd be happy to see support for this (since i'd need it), but as you wrote yourself it would break nearly the whole crate as it is.

IMO the easy way forward, would be to to just add the parsing/writing support for the header and then let the user do whatever they want to do with the body - similar to what i do in the example.

is there anything i can help with?

[ExpHP]

Okay I've almost got everything ironed out now. I'll be pushing something for you to take a look very soon. There is one small thing making it a major semver bump.

[ExpHP]

Please take a look at #82 and see if you have any comments before I merge

[ExpHP]

Huh. It didn't auto detect this as merged. Probably because of using "rebase and merge" on the other. It did give @paroga the contributor badge though, so I can close this one.

[ExpHP]

0.9.0 released