polysemy-research / polysemy-zoo

:monkey::panda_face: Experimental, user-contributed effects and interpreters for polysemy
BSD 3-Clause "New" or "Revised" License
70 stars 20 forks source link

Non type class absorbers #26

Closed adamConnerSax closed 5 years ago

adamConnerSax commented 5 years ago

This is what I have in mind. The code for each absorber is simpler and the typeclass stuff is all gone, since I think all of its cleverness was only useful for the absorbers in the MTL module. This way we are setting a good example in the zoo by doing each one separately and exporting only the constraint-monomorphic absorber. And we are setting a good naming example for people to follow if they write their own absorbers for others to use.

Tests all pass unchanged. The only change there was the name of the test file and the imports. Which is reassuring.

That being said, it's a breaking change.

I'm going to re-write RandomFu's absorber this way and my PandocMonad absorber just to make sure it all works as simply as before, etc.

adamConnerSax commented 5 years ago

Also, I'm not sure why it has so many commits. I thought I had merged the upstream stuff before doing this work. Is that an issue? Is there something I can do on my end to fix it?

adamConnerSax commented 5 years ago

One more question: We don't need to export "using" from Polysemy.ConstraintAbsorber. Should I drop it since absorbWithSem is the version we want people to use since it has the right signature and simpler order of type-variables for use in making absorbers?

isovector commented 5 years ago

As for why the extra commits, I'm not sure. Do you have an upstream remote set on your local repo? I think it might be using the wrong merge strategy, but it's not a concern --- I squash the commits before merging them anyway.

adamConnerSax commented 5 years ago

Also, while we're changing so much, should these things be absorbReader or absorbMonadReader ? It's clear enough here but not all monadic constraints are named MonadXXX``, e.g.,PandocMonadorResource``` or whatever.

isovector commented 5 years ago

I have no strong opinions on the naming. I slightly like absorbReader better due to the brevity.

isovector commented 5 years ago

Does this fix #24 when it lands?

adamConnerSax commented 5 years ago

It doesn't fix it because it wasn't broken. I'll close that one since I found my way through. If we can think of a couple of more standard (than PandocMonad) examples with superclasses or other non-trivial complications, it might be worth putting them in the zoo just so there are more examples for people to look at. Reflection and Constraints are both confusing so using them together is...a lot.

isovector commented 5 years ago

I'm hesitant about PandocMonad due to #16 :)

isovector commented 5 years ago

I'd like to document the fact that this technology exists in polysemy. Do you mind if I write a blog post on it, or do you feel like it would be stealing your thunder? Of course, I'll sing your praise throughout such a post!

adamConnerSax commented 5 years ago

Yeah. I was not suggesting PandocMonad be allowed anywhere near the zoo! Just that we keep an eye out for other examples that might provide inspiration.
PandocMonad was trickier, by a bit, than the reflection example, since the functionality in the PandocMonad part of the dictionary did not cover the MonadError stuff so the dictionary had to be expanded, etc. So if we can find a low-dependency-weight example that seems useful in the zoo, it might be worth adding just for educational reasons.

adamConnerSax commented 5 years ago

Feel free to blog! You figured out much of it anyway. I think I just made you do it... Someday, I'd like to write about these things as well as you do but that day is not now. I was hoping you'd write a post about it since it will get people using it and maybe building more absorbers.

adamConnerSax commented 5 years ago

Also, on an unrelated note, I never remember what "cargo-cult" means. So I looked it up and thought "that makes perfect sense!" and then "and that might be offensive to some since the origin of the term is colonialist condescension to native peoples." and, without much looking, that turns out to be exactly how some people feel. So I think I'll not use it--someone suggested "Religion without faith" instead, which works but is less catchy. Thought it was worth pointing out since I had no idea!

adamConnerSax commented 5 years ago

Not sure if you're waiting for my okay on this, but in case you are, it's good to go from my end. And I have an updated version of polysemy-RandomFu ready to go once this is on hackage.

isovector commented 5 years ago

Fantastic, thanks!