Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'Raise' and 'Subsume' #370

Merged
merged 4 commits into from
Jul 29, 2020
Merged

Add 'Raise' and 'Subsume' #370

merged 4 commits into from
Jul 29, 2020

Conversation

TheMatten
Copy link
Collaborator

@TheMatten TheMatten self-assigned this Jul 28, 2020
@TheMatten TheMatten added enhancement New feature or request polish make the libary joyful t use labels Jul 28, 2020
@tek
Copy link
Member

tek commented Jul 28, 2020

👏

@TheMatten
Copy link
Collaborator Author

raiseNUnder have to stay as they are, because Find can't believe us about effects above being different from each other.

@tek
Copy link
Member

tek commented Jul 28, 2020

I wonder whether a better name for raiseNUnder would be insertAtN…it's not a very intuitive name. or raiseAtN

@TheMatten
Copy link
Collaborator Author

I like @googleson78's realization about what raiseNUnder and raiseUnderN should mean - we could pursue that in v2 in keep it for now as-is in v1. Technically it generalizes too - raiseNUnderM 😜.

@tek
Copy link
Member

tek commented Jul 28, 2020

guess I'm unclear about why it's called raise in the first place

@TheMatten
Copy link
Collaborator Author

I guess this is the reason 😄

@tek
Copy link
Member

tek commented Jul 28, 2020

I see 🙃 I would argue that the name raiseUnder suggests, purely rhetorically, that the e is raised, while it apparently is the r. Not saying that my intuition is a reference here or that I have a better solution, but the UnderN NUnder confusion suggests that there might be room for improvement 🙂

Copy link
Collaborator

@KingoftheHomeless KingoftheHomeless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like magic!

I don't like the names. Something like raiseGeneral and subsumeGeneral would be better.

I'm also slightly concerned about the performance characteristics, but if they turn out to be bad, we can revert the definitions of raiseUnder and subsume and friends to their earlier definitions at a later points.

@KingoftheHomeless
Copy link
Collaborator

KingoftheHomeless commented Jul 29, 2020

Actually, wait. We don't need this, do we? The raiseUnderN class of functions can be achieved through repeated application of raiseUnder. Same thing with repeated raise and subsume. I don't see much point in this if it doesn't also solve the raiseNUnder functions.

@TheMatten
Copy link
Collaborator Author

We can't change raiseNUnder because of how Find works, but we can actually use subsume' in same way at call sites, where top of the stack has concrete effects. So subsume' is still works as raiseNUnder generalization.

When it comes to names, I would prefer something short - that ' can be seen as placeholder for number/postfix (possibly could be replaced with _).

@KingoftheHomeless
Copy link
Collaborator

Oh, well that's fine then.

I'd prefer _ instead of '.

Copy link
Member

@googleson78 googleson78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also thinking whether the typeclasses would be an issue, but I'm hoping that if you use the concrete versions there won't be any (additional) dictionary passing?

@TheMatten
Copy link
Collaborator Author

Even in case with polymorphic "tail", that typeclass duo should resolve into concrete instance - and GHC is really good at inlining instances (see Generics).

@tek
Copy link
Member

tek commented Jul 29, 2020

Oh, well that's fine then.

I'd prefer _ instead of '.

why? isn't _ an indicator of void?

@TheMatten
Copy link
Collaborator Author

@tek I would say that depends a lot on context - and in this context connection to void doesn't make any sense. Instead, I would say _ looks a lot like a "placeholder" for some postfix.

@TheMatten
Copy link
Collaborator Author

Can be considered ready?

Copy link
Collaborator

@KingoftheHomeless KingoftheHomeless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider it so.

Regarding _ vs ': ''s also got a convention, namely, strictness. Neither convention is universally observed: for example, mask_ isn't a void-ed version of mask.

@TheMatten TheMatten merged commit a5d0500 into master Jul 29, 2020
@TheMatten TheMatten deleted the raise-subsume branch July 29, 2020 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request polish make the libary joyful t use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants