Maps should be able to have non-string keys

[bowbahdoe]

In EDN, maps can have keys that are any EDN value.

[naomijub]

Maps do support non string keys, here is an example https://github.com/naomijub/edn-rs/blob/master/examples/navigate_edn.rs#L7 they are just stored as strings for Rust reasons

[bowbahdoe]

{ false   123
  "false" 456 }

I don't think that works, unfortunately.

[naomijub]

The result for this is: Map(Map({"\"false\"": UInt(456), "false": UInt(123)}))

[naomijub]

Do you have a case that actually failed?

[bowbahdoe]

That is a failure though, right? The information in the key is not actually presented in a "parsed" way.

The key in the map for the string with contents false is not a string with contents false, it is the string with contents "false".

    println!(
        "{:?}",
        edn_rs::Edn::from_str("{ :person/name \"bob\" }")
    )
// Ok(Map(Map({":person/name": Str("bob")})))

    println!(
        "{:?}",
        edn_rs::Edn::from_str("{ {:person/name \"bob\"} nil }")
    );
// Ok(Map(Map({"{:person/name \"bob\", }": Nil})))

Like, by sorting the keys it is providing a unique mapping but thats about it

they are just stored as strings for Rust reasons

What reasons?

[bowbahdoe]

I know i'm being basically just a rude stranger, but I think it might bottom out on me just not liking the design/tradeoff taken.

(at which point I might publish my own thing)

    println!(
        "{:?}",
        edn::parse("{ :person/name \"bob\" }")
    );
    // Ok(Map({Keyword(Keyword { namespace: Some("person"), name: "name" }): String("bob")}))
    println!(
        "{:?}",
        edn::parse("{ {:person/name \"bob\"} nil }")
    );
    // Ok(Map({Map({Keyword(Keyword { namespace: Some("person"), name: "name" }): String("bob")}): Nil}))

[naomijub]

Well, you could always contribute to one of the existing edn crates. Some of the trade-off decisions we had to make:

• performance of hashing/indexing edn (maps, sets, lists, vecs)
• usability, in our initial implementation we had two edn indexing possibilities 1) send a String/&str and then crate would parse it and 2) use the associated edn as a key or you would have to manually create an edn and use it as a key. For the usage that we were doing it was better to just have strings that contained the edn value, so false and "false".
• btreemaps/sets instead of hashmaps/sets, because hashing a hashmap/set and a float is a considerable problem, which til this day I don't really like our solution, but you are welcomed to propose a better one. Just a note one consideration we had was having hashmaps saved as Vec<(Key, Edn)>, Key being what ever you think its best, because this would work for hash-maps and for sorted-maps.

[naomijub]

This

 println!(
        "{:?}",
        edn::parse("{ :person/name \"bob\" }")
    );
    // Ok(Map({Keyword(Keyword { namespace: Some("person"), name: "name" }): String("bob")}))

I find a really nice implementation/solution, what do you think @otaviopace ? Also, how is edn-derive on this tagged issue?

Parse the keyword :person/name as namespace: Some("person"), name: "name".

Issue https://github.com/naomijub/edn-rs/issues/84 requires a decision on this as well.

[bowbahdoe]

I ended up publishing what i had as a crate. That might end up being a bad call, but personally i'm not interested in the convenience macro or derive stuff so I don't think there is a path from this crate to what i want

https://github.com/bowbahdoe/edn-format

And as for BTreeMaps, I can't think of a way around it either, but one possible API simplification would be to wrap the BTreeMap so OrderedFloat isn't exposed. Something like

enum ComparableValue {
   Nil
   ... all the same variants
   Float(OrderedFloat(f64)),
   ...
}

pub struct EdnMap {
   inner: BTreeMap<ComparableValue, Value>
}

pub enum Value {
   Nil,
   ...
   Float(f64)
   ...
   Map(EdnMap)
   ...
}

And then when you insert a Value into an EDN map, convert it to a ComparableValue. The same technique could allow for using HashMap or Vec when the keys can all be hashed or when the size of the map is small and then "upgrading" as needed.

enum ComparableValue {
   Nil
   ... all the same variants
   Float(OrderedFloat(f64)),
   ...
}

enum HashableValue {
   Nil
   ... all the same variants
   Float(OrderedFloat(f64)),
   ... but without those that can't be hashed
   ...
}

enum MapImpl<V> {
    Array(Vec<(Value, V)>),
    Hash(HashMap<HashableValue, V>),
    BTree(BTreeMap<ComparableValue, V>)
}

pub struct EdnMap {
   inner: MapImpl<Value>
}

pub struct EdnSet {
   inner: MapImpl<()>
}

pub enum Value {
   Nil,
   ...
   Float(f64)
   ...
   Map(EdnMap)
   ...
}

But I think some profiling would be in order to say if any of that makes sense

[naomijub]

We are avoiding other crates usage, but you gave me another idea

[naomijub]

well this is solved by issue #88