Tidal
Tidal copied to clipboard
Define the Eq and Ord instances for the Event type by hand.
This way, otherwise-equal Event
s are considered equal
even if their Context
s (debugging information) differ.
Also, Event
s are compared first on when they happen
(the part
field), second on their value
,
and only last on their whole
field (which is often Nothing
).
Sorting a List
of Event
s will thus cause the early ones
to be first in the List
, and contemporaneous ones
to be adjacent in the list even if their whole
s differ.
I feel a bit uneasy about this. The other suggested change (remove Context from instance Show Event) is just for human-readable output, but this here concerns ==
(etc.) that may be important for code. Are we sure that actual (derived) equality is never used (now, and in the future)?
The main motivation is for writing tests? There are are already some classes/functions for comparing (sets of) events, defined in https://github.com/tidalcycles/Tidal/blob/669fde100a04ed1f86460a2f38ca618aa8d12150/test/TestUtils.hs#L34 (TolerantEq, compareP, comparePD). How does this relate?
If there are some Events where Context is important, and some where it is not, then the True Haskell Way is to make them distinct types, so they cannot be confused, and each can have their specific class instance(s). Or
data EventF c a b = EventF { context :: c, part :: a, value :: b, ... }
The function you wrote
relevant e = (whole e, part e, value e)
actually wants to have type EventF c a b -> EventF () a b
.
An intermediate step (that does not break anything) would be to just hoist it to top level, and export it, perhaps as
withoutContext :: Event a -> Event a
withoutContext e = e { context = [] }
Re: are we sure that derived Eq is never used
I commented out the deriving (Eq, Ord)
clause, this creates an error in defragParts
because of
src/Sound/Tidal/Pattern.hs:586:57: error:
• Could not deduce (Eq (Event a)) arising from a use of ‘delete’
in this code
defragParts (e:es) | isJust i = defraged : defragParts (delete e' es)
...
where i = findIndex (isAdjacent e) es
e' = es !! fromJust i
Here, delete
is unnecessary (so we don't need instance Eq (Event a))
since we already know the index of the element to be removed, so a function like deleteAt
https://hackage.haskell.org/package/containers-0.6.5.1/docs/Data-Sequence.html#v:deleteAt
should be used (but it seems Data.List does not have it)
Then, the Eq and Ord instances are used in
compareDefrag :: (Ord a) => [Event a] -> [Event a] -> Bool
compareDefrag as bs = sort (defragParts as) == sort (defragParts bs)
Is this supposed to respect Contexts, or to ignore them? The only application I found is in
comparePD
(used in tests)
comparePD a p p' = compareDefrag es es'
where es = query (stripContext p) (State a Map.empty)
es' = query (stripContext p') (State a Map.empty)
A-ha, the proposed context-removal function did exist all along. Well, for patterns it did.
stripContext = setContext $ Context []
setContext c pat = withEvents (map (\e -> e {context = c}))
Let's give a name (withoutContext
, or similar) to this anonymous function here.
stripContext
is certainly a handy thing to be able to do.
I wouldn't be terribly bothered if the changes to Eq
didn't make it through. In fact even keeping context
part of the Ord
instance could be acceptable -- if it were considered last among fields, rather than first (as it is now).