-
Notifications
You must be signed in to change notification settings - Fork 48
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
append and split NP #115
base: master
Are you sure you want to change the base?
append and split NP #115
Conversation
Add functions to append and split n-ary products.
sop-core/src/Data/SOP/NP.hs
Outdated
@@ -10,6 +15,8 @@ module Data.SOP.NP | |||
, pure_POP | |||
, cpure_NP | |||
, cpure_POP | |||
, (++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually exporting the term-level (++)
from the Prelude
, not the type-level (++)
that you defined. I think you intended to export type (++)
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Fixed! I'm not dedicated to the names btw.
Thanks! Could you provide a bit of motivation of why these functions are required / what your use case is? I think @phadej wanted to add something similar. Perhaps he could have a look whether the proposed changes are sufficient for his purposes. The general question I'm uncertain about is whether functions that presumably have no purpose for the "generics" application of n-ary sums and products should go into Regarding the actual contribution: according to current naming conventions, the functions should not be called |
append and split make sense for append_NS :: Either (NS f xs) (NS f ys) -> NS f (Append xs ys) And indeed, I have |
I need the pad_NS :: NS f xs -> proxy ys -> NS f (xs ++ ys) |
@phadej Right, I agree that having What are the other functions you have? I have to read up on #116, but I can certainly be convinced to just add these functions to |
Few, but only (++)
concat
\xss yss -> concatMap (\xs -> map (xss ++) yss) xss -- (SOP f xss, SOP f yss) <-> SOP f (ConcatMapAppend xss yss), for each xs in xss and ys in yss there's (xs ++ ys) in ConcatMapAppend xss yss
concatMap (map concat . sequence) -- SOP (SOP f) xssss <-> SOP f (FLATTEN xsssss)
liftA2 (:)
\xss yss -> map (xs ++) yss
map concat
\x ys -> map (x :) ys
sequence @[] @{] -- NP (NS f) xss <-> NS (NP f) (Sequence xss) E.g. I'm not sure whether these hasochism artifacts should just be kept secret, and people should use |
Side-note: I'd prefer using data a & b = Pair a b -- which is IMHO just causes confusion. I'd like to use (~>) as a type-variable to define my Arrows). |
Ok, I think I'm nearly convinced that we should just add The concrete plan seems to be:
On the naming issue, I think I also prefer |
@kosmikus My logical argument is that Anyway, back to topic, it's clear where to put |
How do you want to capture append_NP :: NP f xs -> NP f ys -> NP f (xs ++ ys)
append_NS :: Either (NS f xs) (NS f ys) -> NS f (xs ++ ys) It's possible by also adding a class HSplit prod h where
happend :: prod (h f xs) (h f ys) -> h f (xs ++ ys)
hsplit :: SListI xs => h f (xs ++ ys) -> prod (h f xs) (h f ys)
instance HSplit (,) NP
instance HSplit Either NS And then |
@echatav we do something like that for I argue that Those are details which should be easy enough to polish on the PR. |
I agree it would be nicer to have the curried variant of I have in general started to almost always use the non-overloaded versions of the So for the PR, we should at the very least add all of |
Add functions to append and split n-ary products.