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

dunai: Remove deprecated definitions #445

Closed
ivanperez-keera opened this issue Oct 22, 2024 · 10 comments
Closed

dunai: Remove deprecated definitions #445

ivanperez-keera opened this issue Oct 22, 2024 · 10 comments

Comments

@ivanperez-keera
Copy link
Owner

dunai includes several deprecated definitions that were deprecated three versions ago (see #418). They can safely be removed.

@ivanperez-keera
Copy link
Owner Author

ivanperez-keera commented Oct 22, 2024

If someone wants to do this, please go ahead and send a PR.

Please create 5 commits:

  • Commit 1: Remove all deprecated definitions from dunai/src/Control/Monad/Trans/MSF/List.hs. Remove also any conditional imports of Control.Monad.Trans.List, and the OPTIONS_GHC at the top. Adjust also the module comment that talks about ListT's constraints.
  • Commit 2: Modify the cabal package definition to make depending on list-transformer not optional. Remove the flag list-transformer.
  • Commit 3: Update the example in dunai-examples/list/ to use list-transformer.
  • Commit 4: Update the examples in dunai-examples/paper to use list-transformer.
  • Commit 5: Update dunai's changelog.

Please check out the recent history of this repository to see how we normally document commits, and how we link commits to issues in the subject line of the commit messages.

@ivanperez-keera ivanperez-keera changed the title Remove deprecated definitions from dunai dunai: Remove deprecated definitions Nov 3, 2024
@solomon-b
Copy link
Contributor

There are a few issues with bearriver-examples-bouncingball-list.

  1. Not included in cabal.project
  2. Ambiguous imports:
BouncingBall.hs:50:40: error:
    Ambiguous occurrence ‘arr2’
    It could refer to
       either ‘Yampa.arr2’,
              imported from ‘FRP.Yampa’ at BouncingBall.hs:20:1-52
              (and originally defined in ‘FRP.BearRiver.Arrow’)
           or ‘Main.arr2’, defined at BouncingBall.hs:121:1
   |
50 |               in (oldfb &&& newfb) >>> arr2 (++)

Mind if I adjust the import qualifiers?

@solomon-b
Copy link
Contributor

solomon-b commented Nov 3, 2024

Oh we can just remove arr2, but there is another problem:

BouncingBall.hs:32:15: error: [GHC-83865]
    • Couldn't match type ‘Identity’ with ‘IO’
      Expected: bearriver-0.14.11:FRP.BearRiver.InternalCore.SF
                  IO (Float, Float, Bool, Bool) [(Float, Float)]
        Actual: MSF
                  (ClockInfo Identity) (Float, Float, Bool, Bool) [(Float, Float)]
    • In the fourth argument of ‘reactimate’, namely ‘bouncingBalls’
      In a stmt of a 'do' block:
        reactimate
          (getMouse) (\ _ -> getMouse >>= (\ p -> return (0.02, Just p)))
          (\ _ e -> render e >> return False) bouncingBalls
      In the expression:
        do SDL.init [InitEverything]
           setVideoMode 800 600 32 [SWSurface]
           reactimate
             (getMouse) (\ _ -> getMouse >>= (\ p -> return (0.02, Just p)))
             (\ _ e -> render e >> return False) bouncingBalls
   |
32 |               bouncingBalls
   |               ^^^^^^^^^^^^^

Do we want the Yampa or Dunai reactimate?

@ivanperez-keera
Copy link
Owner Author

ivanperez-keera commented Nov 3, 2024

For the first problem, since it's technically unrelated to this issue, I've opened a separate issue to track it:

#452

I suggest you send a PR for #452 first, and then send a PR for this.

I don't want to add it to the cabal.project yet.

@ivanperez-keera
Copy link
Owner Author

For the second issue, it sounds like maybe bearriver's FRP.Yampa module should offer a reactimate that works on any monad and assumes the SF to be SF Identity a b instead of SF m a b where the m has to match the return monad of reactimate. If so, that should also be filed as a separate issue.

Let met think about that for a bit and come back to this.

Thanks for the great catch!

@solomon-b
Copy link
Contributor

solomon-b commented Nov 3, 2024

I'm not sure if this is correct but you could recursively runIdentity and pure the SF continuations to get into your arbitrary monad:

reactimate :: Monad m
           => m a                          -- ^ Initialization action
           -> (Bool -> m (DTime, Maybe a)) -- ^ Input sensing action
           -> (Bool -> b -> m Bool)        -- ^ Actuation (output processing)
                                           --   action
           -> SF a b                       -- ^ Signal function
           -> m ()
reactimate senseI sense actuate sf = do
    MSF.reactimateB $ senseSF >>> foldIdentity sfIO >>> actuateSF
    return ()
  where

    foldIdentity :: Monad m => MSF Identity (DTime, a) b ->  MSF m (DTime, a) b
    foldIdentity (MSF k) = MSF $ \ta -> pure $ fmap foldIdentity $ runIdentity $ k ta

    sfIO = MSF.runReaderS sf

    -- Sense
    senseSF = MSF.dSwitch senseFirst senseRest

    -- Sense: First sample
    senseFirst = constM senseI >>> arr (\x -> ((0, x), Just x))

    -- Sense: Remaining samples
    senseRest a = constM (sense True) >>> (arr id *** keepLast a)

    keepLast :: Monad m => a -> MSF m (Maybe a) a
    keepLast a = MSF $ \ma ->
      let a' = fromMaybe a ma
      in a' `seq` return (a', keepLast a')

    -- Consume/render
    actuateSF = arr (\x -> (True, x)) >>> arrM (uncurry actuate)

@ivanperez-keera
Copy link
Owner Author

I wonder if it's possible to also do it by wrapping the identity-based SF into one in an arbitrary monad with return . runIdentity and use:

https://hackage.haskell.org/package/dunai-0.13.2/docs/Data-MonadicStreamFunction-Core.html#v:morphS

@ivanperez-keera
Copy link
Owner Author

Could you do something like:

reactimate :: Monad m
           => m a                          -- ^ Initialization action
           -> (Bool -> m (DTime, Maybe a)) -- ^ Input sensing action
           -> (Bool -> b -> m Bool)        -- ^ Actuation (output processing)
                                           --   action
           -> SF a b                       -- ^ Signal function
           -> m ()
reactimate senseI sense actuate sf =
    BearRiver.reactimate senseI sense actuate (foldIdentity sf)
  where
    foldIdentity :: Monad m => MSF Identity (DTime, a) b ->  MSF m (DTime, a) b
    foldIdentity (MSF k) = MSF $ \ta -> pure $ fmap foldIdentity $ runIdentity $ k ta

or

reactimate :: Monad m
           => m a                          -- ^ Initialization action
           -> (Bool -> m (DTime, Maybe a)) -- ^ Input sensing action
           -> (Bool -> b -> m Bool)        -- ^ Actuation (output processing)
                                           --   action
           -> SF a b                       -- ^ Signal function
           -> m ()
reactimate senseI sense actuate sf =
    BearRiver.reactimate senseI sense actuate (foldIdentity sf)
  where
    foldIdentity :: Monad m => MSF Identity (DTime, a) b ->  MSF m (DTime, a) b
    foldIdentity = morphS (return . runIdentity)

@solomon-b
Copy link
Contributor

Oh nice, I hadn't seen morphS. Do you want this new version of reactimate in its own PR?

@ivanperez-keera
Copy link
Owner Author

ivanperez-keera commented Nov 4, 2024

@solomon-b Yes. I've just opened a separate issue #454 to address that concern specifically.

ivanperez-keera added a commit that referenced this issue Dec 23, 2024
dunai includes several deprecated definitions that were deprecated three
versions ago when support for using list-transformer (instead of list-t)
was introduced. As per our policy of waiting three versions since
deprecation until a function is removed, these definitions can be safely
removed.

This commit removes the deprecated functions, changes the GHC options
and all conditionals related to picking different versions of ListT, and
all related imports. The documentation is adjusted to remove the mention
on the list monad having constraints on the monad it is applied to.
ivanperez-keera added a commit that referenced this issue Dec 23, 2024
A prior commit has made using list-transformer the only option available
for a ListT monad due to the older interface having been deprecated more
than three versions ago.

This commit adjusts dependencies so that that package list-transformer
is always picked, and removes the related flag since it is now no longer
optional.
ivanperez-keera added a commit that referenced this issue Dec 23, 2024
A prior commit has made using list-transfomer the library providing an
implementation of ListT used by dunai.

This commit adjusts an example to use the same interface. We further
adjust the use of the function widthFirst instead of a custom
implementation of equivalent behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants