Skip to content

Commit a218c2d

Browse files
tbidnetbidne
committed
Allow distance meters in charts
1 parent fa256c0 commit a218c2d

6 files changed

Lines changed: 241 additions & 21 deletions

File tree

‎src/Pacer/Command/Chart/Data/ChartData.hs‎

Lines changed: 67 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,11 @@ type AccY = NESeq (Tuple2 Timestamp Double)
169169
-- | Accumulator for chart with two Y axes.
170170
type AccY1 = NESeq (Tuple3 Timestamp Double Double)
171171

172+
hasPace :: ChartData -> Bool
173+
hasPace (ChartDataY y) = y ^. #yType == YAxisPace
174+
hasPace (ChartDataY1 y) =
175+
(y ^. #yType == YAxisPace) || (y ^. #y1Type == YAxisPace)
176+
172177
-- | Turns a sequence of activities and a chart request into a chart.
173178
mkChartData ::
174179
forall es a.
@@ -197,7 +202,7 @@ mkChartData finalDistUnit globalFilters as request = do
197202
Empty -> pure $ Err $ CreateChartFilterEmpty $ request ^. #title
198203
r :<| rs -> do
199204
activities <- handleChartType (request ^. #chartType) (r :<|| rs)
200-
pure $ Ok $ mkChartDataSets finalDistUnit request activities
205+
Ok <$> mkChartDataSets finalDistUnit request activities
201206
where
202207
(MkSomeActivities (SetToSeqNE someActivities)) = as
203208
filteredActivities =
@@ -295,8 +300,9 @@ handleChartType @es @a mChartType someActivities = case mChartType of
295300
toDatetime = view #datetime
296301

297302
mkChartDataSets ::
298-
forall a.
303+
forall a es.
299304
( Fromℤ a,
305+
Logger :> es,
300306
Ord a,
301307
Semifield a,
302308
Show a,
@@ -305,17 +311,29 @@ mkChartDataSets ::
305311
DistanceUnit ->
306312
ChartRequest a ->
307313
NESeq (SomeActivity a) ->
308-
ChartData
309-
mkChartDataSets @a finalDistUnit request activities@(MkSomeActivity sd _ :<|| _) =
310-
case request ^. #y1Axis of
311-
Nothing ->
312-
let vals = withSingI sd $ foldMap1 toAccY activities
313-
yType = request ^. #yAxis
314-
in ChartDataY (MkChartY vals yType finalDistUnit)
315-
Just y1Type ->
316-
let vals = withSingI sd $ foldMap1 (toAccY1 y1Type) activities
317-
yType = request ^. #yAxis
318-
in ChartDataY1 (MkChartY1 vals yType y1Type finalDistUnit)
314+
Eff es ChartData
315+
mkChartDataSets @a finalDistUnit request activities@(MkSomeActivity sd _ :<|| _) = do
316+
let chartData = case request ^. #y1Axis of
317+
Nothing ->
318+
let vals = withSingI sd $ foldMap1 toAccY activities
319+
yType = request ^. #yAxis
320+
in ChartDataY (MkChartY vals yType finalDistUnit)
321+
Just y1Type ->
322+
let vals = withSingI sd $ foldMap1 (toAccY1 y1Type) activities
323+
yType = request ^. #yAxis
324+
in ChartDataY1 (MkChartY1 vals yType y1Type finalDistUnit)
325+
326+
-- Log a message if meters and pace were requested.
327+
when (hasPace chartData && finalDistUnit == Meter) $ do
328+
let msg =
329+
mconcat
330+
[ "Requested meters for chart '",
331+
request ^. #title,
332+
"', converting pace to km."
333+
]
334+
$(Logger.logInfo) msg
335+
336+
pure chartData
319337
where
320338
toAccY :: SomeActivity a -> AccY
321339
toAccY sr@(MkSomeActivity _ r) = NESeq.singleton (r ^. #datetime, toY sr)
@@ -331,18 +349,14 @@ mkChartDataSets @a finalDistUnit request activities@(MkSomeActivity sd _ :<|| _)
331349
toYHelper axisType (MkSomeActivity s r) = case axisType of
332350
YAxisDistance ->
333351
withSingI s $ case finalDistUnit of
334-
-- REVIEW: Should this be (DistU.convertDistance Meter)? Or do we
335-
-- only allow km/mi in charts?
336-
Meter -> toDistRaw (DistU.convertToKilometers r)
352+
-- See NOTE: [Chart Units]
353+
Meter -> toDistRaw (DistU.convertDistance Meter r)
337354
Kilometer -> toDistRaw (DistU.convertToKilometers r)
338355
Mile -> toDistRaw (DistU.convertDistance Mile r)
339356
YAxisDuration -> toDurRaw r
340357
YAxisPace ->
341358
withSingI s $ case finalDistUnit of
342-
-- REVIEW: Ideally this would error, not convert. But probably we
343-
-- already throw errors somewhere, and the Meters are only here
344-
-- because we haven't proven to GHC that it is impossible. It would
345-
-- be nice to come up with something more robust.
359+
-- See NOTE: [Chart Units]
346360
Meter -> toPaceRaw (DistU.convertToKilometers r)
347361
Kilometer -> toPaceRaw (DistU.convertToKilometers r)
348362
Mile -> toPaceRaw (DistU.convertDistance Mile r)
@@ -356,6 +370,39 @@ mkChartDataSets @a finalDistUnit request activities@(MkSomeActivity sd _ :<|| _)
356370
toDurRaw :: Activity d a -> Double
357371
toDurRaw = toℝ . view (#duration % #unDuration)
358372

373+
-- NOTE: [Chart Units]
374+
--
375+
-- In general, the frontend supports all units as the y-axis:
376+
--
377+
-- distance: m, km, mi
378+
-- duration: <time>
379+
-- pace: m, km, mi
380+
--
381+
-- However, we explicitly do not allow pace to be used with meters, so the
382+
-- question of what to do if the user specifies 'meters' in a chart request
383+
-- (charts[i].unit = "m") arises.
384+
--
385+
-- Previously, we converted both YAxisDistance and YAxisPace to km, so no
386+
-- warnings/errors, but only km was allowed. We now decide to let distance
387+
-- respect meters, though pace is still converted. We do this for several
388+
-- reasons:
389+
--
390+
-- - In general, we should respect the user's choice when we can, and meters
391+
-- are perfectly sensible for some distances (e.g. track events).
392+
--
393+
-- - We should allow graphing distance by meters and pace by km, but there is
394+
-- only one distance unit in chart-requests at the moment, so pace cannot
395+
-- then error.
396+
--
397+
-- - If we ever add something like elevation, meters makes sense there as
398+
-- well.
399+
--
400+
-- Hence we log a message for conversions, but otherwise respect the choice.
401+
-- This raises the possibility of specifying multiple units e.g.
402+
-- distance_unit and pace_unit, because as of now we cannot have
403+
-- distance in meters and pace in miles, which is also reasonable. But that
404+
-- is a low priority.
405+
359406
-- | A moving window has the group representative be the median element.
360407
smoothWindow ::
361408
( Fromℤ a,

‎src/Pacer/Prelude.hs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ module Pacer.Prelude
1919
-- * Tuples
2020
uncurry3,
2121

22+
-- * Foldable
23+
foldMapA,
24+
foldMapA1,
25+
2226
-- * ByteString
2327
-- ** Lazy
2428
LazyByteString,
@@ -204,7 +208,7 @@ import Data.Maybe as X
204208
isJust,
205209
maybe,
206210
)
207-
import Data.Monoid as X (Monoid (mconcat, mempty))
211+
import Data.Monoid as X (Ap (Ap, getAp), Monoid (mconcat, mempty))
208212
import Data.Ord as X
209213
( Ord (compare, (<), (<=), (>), (>=)),
210214
Ordering (EQ, GT, LT),
@@ -705,3 +709,9 @@ identity x = x
705709

706710
uncurry3 :: (a -> b -> c -> d) -> Tuple3 a b c -> d
707711
uncurry3 f (a, b, c) = f a b c
712+
713+
foldMapA :: (Applicative m, Foldable t, Monoid b) => (a -> m b) -> t a -> m b
714+
foldMapA f = getAp <$> foldMap (Ap . f)
715+
716+
foldMapA1 :: (Applicative m, Foldable1 t, Semigroup b) => (a -> m b) -> t a -> m b
717+
foldMapA1 f = getAp <$> foldMap1 (Ap . f)

‎test/functional/Functional/Pacer/Command/Chart.hs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ basicTests getTestDir =
2222
[ testExampleChart getTestDir,
2323
testSimple getTestDir,
2424
testSimpleBuildDir getTestDir,
25+
testChartUnitMeters getTestDir,
2526
testFilter getTestDir,
2627
testFilterPreds getTestDir,
2728
testFilterDates getTestDir,
@@ -96,6 +97,35 @@ testSimpleBuildDir getTestDir = testGoldenParams getTestDir params
9697
dataDir =
9798
unsafeDecode [ospPathSep|test/functional/data/testSimpleBuildDir|]
9899

100+
testChartUnitMeters :: IO OsPath -> TestTree
101+
testChartUnitMeters getTestDir = testGoldenParams getTestDir params
102+
where
103+
params =
104+
MkGoldenParams
105+
{ mkArgs = \p ->
106+
[ "--log-level",
107+
"info",
108+
"chart",
109+
"--json",
110+
"--data",
111+
dataDir,
112+
"--build-dir",
113+
buildDir p
114+
],
115+
outFileName =
116+
GoldenOutputFileAssertLogs
117+
[ospPathSep|build/charts.json|]
118+
checkLog,
119+
testDesc = "Chart unit meters",
120+
testName = [osp|testChartUnitMeters|]
121+
}
122+
buildDir p = unsafeDecode $ p </> [osp|build|]
123+
dataDir = unsafeDecode [ospPathSep|test/functional/data/testChartUnitMeters|]
124+
125+
checkLog =
126+
T.isInfixOf
127+
"Requested meters for chart 'Runs in m', converting pace to km."
128+
99129
testFilter :: IO OsPath -> TestTree
100130
testFilter = testChart "Filter example" [osp|testFilter|]
101131

‎test/functional/data/testChartUnitMeters/activities.json‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"activities": [
3+
{
4+
"datetime": "2024-10-15",
5+
"distance": "5 km",
6+
"duration": "20m30s",
7+
"type": "Running"
8+
},
9+
{
10+
"datetime": "2024-10-20T14:30:00",
11+
"distance": "20 miles",
12+
"duration": "2h40m54s",
13+
"labels": [],
14+
"type": "Running"
15+
},
16+
{
17+
"datetime": "2024-10-25T12:00:00-08:00",
18+
"distance": "marathon",
19+
"duration": "3h20m",
20+
"labels": ["official", "marathon"],
21+
"type": "Running"
22+
},
23+
{
24+
"datetime": "2024-10-26",
25+
"distance": "half-marathon",
26+
"duration": "1h40m",
27+
"labels": ["official", "half-marathon"],
28+
"title": "Some half marathon",
29+
"type": "Running"
30+
}
31+
]
32+
}

‎test/functional/data/testChartUnitMeters/chart-requests.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"charts": [
3+
{
4+
"title": "Runs in km",
5+
"y-axis": "distance",
6+
"y1-axis": "pace",
7+
"description": "Simple charts 1"
8+
},
9+
{
10+
"title": "Runs in m",
11+
"y-axis": "distance",
12+
"y1-axis": "pace",
13+
"description": "Simple charts 2",
14+
"unit": "m"
15+
}
16+
]
17+
}
18+

‎test/functional/goldens/testChartUnitMeters.golden‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
build/charts.json
2+
3+
{
4+
"charts": [
5+
{
6+
"datasets": {
7+
"xAxis": [
8+
"2024-10-15",
9+
"2024-10-20T14:30:00",
10+
"2024-10-25T12:00:00-08:00",
11+
"2024-10-26"
12+
],
13+
"yAxes": {
14+
"y": {
15+
"data": [
16+
5,
17+
32.18,
18+
42.195,
19+
21.0975
20+
],
21+
"id": "y",
22+
"label": "km",
23+
"type": "distance"
24+
},
25+
"y1": {
26+
"data": [
27+
246,
28+
300,
29+
284.3938855314611,
30+
284.3938855314611
31+
],
32+
"id": "y1",
33+
"label": "pace /km",
34+
"type": "pace"
35+
}
36+
}
37+
},
38+
"extra": {
39+
"description": "Simple charts 1"
40+
},
41+
"title": "Runs in km"
42+
},
43+
{
44+
"datasets": {
45+
"xAxis": [
46+
"2024-10-15",
47+
"2024-10-20T14:30:00",
48+
"2024-10-25T12:00:00-08:00",
49+
"2024-10-26"
50+
],
51+
"yAxes": {
52+
"y": {
53+
"data": [
54+
5000,
55+
32180,
56+
42195,
57+
21097.5
58+
],
59+
"id": "y",
60+
"label": "m",
61+
"type": "distance"
62+
},
63+
"y1": {
64+
"data": [
65+
246,
66+
300,
67+
284.3938855314611,
68+
284.3938855314611
69+
],
70+
"id": "y1",
71+
"label": "pace /m",
72+
"type": "pace"
73+
}
74+
}
75+
},
76+
"extra": {
77+
"description": "Simple charts 2"
78+
},
79+
"title": "Runs in m"
80+
}
81+
]
82+
}
83+

0 commit comments

Comments
 (0)