Support serializing of maps

[birktj]

I want to use this library in order to import/export csv files to/from a key-value database, I use HashMap<String, MyValueType> to store this data. This works fine for deserializing, however I just discovered that it was not supported for serializing, something which feels like an inconsistency in the library. This is a feature that I would appreciate.

[BurntSushi]

I don't think this is a matter of will, but rather, a missing specification. If someone wants this feature, then they need to propose how it should work, in detail. A principle question that must be answered is the ordering of columns (which ordering?) and whether or not it should be deterministic (and if so, how).

[BurntSushi]

(IMO, I think a valid answer to this question is "the library doesn't support it and callers should convert their data type to something else that is already supported." If you're creating a hash map for every record, then you've already given up on making it fast, so the cost of an extra conversion step is just a little more code in the form of a small helper funciton.)

[birktj]

You are probably right, I just realized that this was just the push I needed to actually implement a kind of sorted hashmap so that I can get the csv out in the same format that I inserted it in. However it still seems like an inconsistency with support only for deserializing of maps. I don't think that serialization of at least hashmaps has to be deterministic, when you first choose to use them you are probably no longer interested in ordering.

[BurntSushi]

I don't really buy inconsistency arguments just for the sake of consistency, particularly when there are complications. Reading into a hash map and writing from a hash map have fundamentally different concerns when it comes to CSV data.

I also don't really agree about determinism. Non-determinism is often surprising when you hit it, and while it is appropriate for HashMap to have arbitrary order, I wouldn't want to propagate that out further.

The reason why I think the status quo is a reasonable option is because it allows the library to not make a choice about determinism and ordering, and forces the caller to do it.

With that said, here are other thoughts:

1. It seems like BTreeMap should be allowed, since the ordering is made clear by the type.
2. It may be worthwhile to attempt to pop up a level and focus on the problem you're trying to solve. If the problem is, "I want a simple way to read CSV data without caring much about its structure, but still retain the ability to inspect it dynamically via header field names," (which I'm assuming is what you're doing if you're using a HashMap), then there are other approaches here. For example, we might consider a new record type that retains ordering information and exposes a map-like API.

[synek317]

I think that serializing maps may be required in order to work with #[serde(flatten)] attribute.

[Kampfkarren]

@synek317 It is, which leads me to going on this issue. I need to have flattened stuff which should work completely fine in a CSV format.

[BurntSushi]

I don't see why serializing maps is required in order for serde(flatten) to work. Can you provide an example?

[Kampfkarren]

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=8312d6a3ee38bdaed677865367405830

Removing the baz: Baz bit serializes fine.

[BurntSushi]

Interesting. I didn't expect that. Thanks.

…

On Sun, Nov 18, 2018, 01:10 Kampfkarren ***@***.*** wrote: https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=8312d6a3ee38bdaed677865367405830 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#98 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAb34pBCiPuqjP9lfAqtIMm609ZXXKhVks5uwPnUgaJpZM4QjLWL> .

[Kampfkarren]

Would you know how to solve it? It's a complete blocker of this right now for me 😕

[Kampfkarren]

Kampfkarren@95cea44

This makes flattened maps work, but probably gives weird behavior for unflattened maps which is why I'm not PRing it.

[BurntSushi]

@Kampfkarren I don't know off the top of my head. I'm not familiar with how serde(flatten) works. I would have thought that it would just generate more code at compile time as if everything was a struct instead of using something dynamic like a map. Moreover, I'm fairly certain that serde(flatten) worked in the csv crate at one point in time, so I'd want to do a bit of code archaeology to figure out what went wrong.

It's a complete blocker of this right now for me

I don't understand this either. serde(flatten) is I'm sure very useful, but AIUI, it's still just a convenience.

This makes flattened maps work, but probably gives weird behavior for unflattened maps which is why I'm not PRing it.

Right. If making serde(flatten) work requires supporting arbitrary maps work, then we need to nail down a specification for maps first.