search

noprompt / meander

Tools for transparent data transformation
MIT License
923 stars 55 forks source link

Adds ability for all to work on records. #54

Closed jimmyhmiller closed 5 years ago

jimmyhmiller commented 5 years ago

Sadly, because records are also maps, I had to add it as a special case.

This sort of functionality is important when working with code built around records, I ran into it when working with test.check generators.

noprompt commented 5 years ago

Could you extend records with the other protocols as well? If you want to do that separately that’s okay with me but we may as well do the others too.

jimmyhmiller commented 5 years ago

I tried "one" and it didn't seem to have the same issue. Not familiar with the others to know if they have similar problems.

noprompt commented 5 years ago

Ah okay. If there’s no issue with records and the other protocols then you’re free to merge this. 👍

jimmyhmiller commented 5 years ago

Some and retain might have the problem. I will try to look into them.

jimmyhmiller commented 5 years ago

So I made this work with some as well. I don't think that retain makes sense for records.

First, there is an analogy here to monoids. The thing I change for all and some were their empty case. Before we had maps, now I used the record itself. We then are appending to that record as we reduce.

Using the record itself works for all and some because we aren't changing structure. With retain, we are removing things. So we really need to start out with the mempty of that type. But we don't have such a thing for records.

Second, records can't have fewer than their declared fields so retain could only keep the type if all properties matched.

one doesn't have these problems because it uses fail as the base case, not an empty.

Just want to double check that my thinking here is right before merging.